Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Andy Shevchenko @ 2026-03-19  8:21 UTC (permalink / raw)
  To: Jason Yan
  Cc: Sam Ravnborg, Helge Deller, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <e5f7611f-f087-4835-99e3-4fddc927aab8@huawei.com>

On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> > > 在 2026/3/18 15:45, Andy Shevchenko 写道:
> > > > On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
> > > > > On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> > > > > > Fix the following gcc warning:
> > > > > > 
> > > > > > drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> > > > > > ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
> > > > > >     unsigned int pixel_vco;
> > > > > >                  ^~~~~~~~~
> > > > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > > > > Signed-off-by: Jason Yan <yanaijie@huawei.com>
> > > > > 
> > > > > Thanks, committed and pushed to drm-misc-next.
> > > > > The fix will show up in upstream kernel at the next
> > > > > merge window.
> > > > 
> > > > The most of the patches from so called Hulk Robot appeared to be controversial.
> > > > 
> > > > First of all, even so called "dead code" may have side effects on the registers
> > > > in HW which may lead to other issues. Second, the mentioned dead code elimination
> > > > patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> > > > I got into that, I still have a build error).
> > > 
> > > Sorry, I do not understand what side effects this may cause. Would you
> > > please explain it in detail?
> > 
> > Any IO (and specifically on PCI bus) may have side effects. If the removed code
> > did some HW accesses (especially reads), it affects the state of HW. You can
> > read more about PCI bus and what read from it does.
> 
> No, the removed code did not do any IO and will not affect the state of HW.

Excellent, but it leaves the code that does IO and now still breaks the
compilation.

> > Helge, can we revert this change and start over, please? (I can also send
> > revert if you think it's a better way)
> 
> I don't think we need to revert this patch. Please provide proof that this
> modification will lead to real issues.

It's not me, who should provide an evidence, it had to be the author
who must had provided testing and everything when submitting the change.

> > I do not believe this change was ever tested on real HW, and I see an evidence
> > that this most likely was blindly made as it led to the similar issue after
> > the change.

No answer here? That's what I expected based on my observations on the quality
of the patches from so called Hulk Robot.

> > > > That said, for the starter I suggest to revert this change. After one need go
> > > > carefully through code to understand if it's exactly the case and what to do with
> > > > 'mnp' which involves some IO.
> > > > 
> > > > +Cc: current fbdev maintainers

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Helge Deller @ 2026-03-19  8:35 UTC (permalink / raw)
  To: Andy Shevchenko, Jason Yan
  Cc: Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <abuyJDK6E2aHA4rC@ashevche-desk.local>

Hi Andy,

On 3/19/26 09:21, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
>> 在 2026/3/19 15:38, Andy Shevchenko 写道:
>>> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
>>>> 在 2026/3/18 15:45, Andy Shevchenko 写道:
>>>>> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>>>>>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>>>>>> Fix the following gcc warning:
>>>>>>>
>>>>>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>>>>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>>>>>      unsigned int pixel_vco;
>>>>>>>                   ^~~~~~~~~
>>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>>>>>
>>>>>> Thanks, committed and pushed to drm-misc-next.
>>>>>> The fix will show up in upstream kernel at the next
>>>>>> merge window.
>>>>>
>>>>> The most of the patches from so called Hulk Robot appeared to be controversial.
>>>>>
>>>>> First of all, even so called "dead code" may have side effects on the registers
>>>>> in HW which may lead to other issues. Second, the mentioned dead code elimination
>>>>> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
>>>>> I got into that, I still have a build error).
>>>>
>>>> Sorry, I do not understand what side effects this may cause. Would you
>>>> please explain it in detail?
>>>
>>> Any IO (and specifically on PCI bus) may have side effects. If the removed code
>>> did some HW accesses (especially reads), it affects the state of HW. You can
>>> read more about PCI bus and what read from it does.
>>
>> No, the removed code did not do any IO and will not affect the state of HW.
> 
> Excellent, but it leaves the code that does IO and now still breaks the
> compilation.
> 
>>> Helge, can we revert this change and start over, please? (I can also send
>>> revert if you think it's a better way)

Andy, all points you make against removing relevant code is absolutely right.

But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
set but not used variable") I have to agree with Jason, that the patch is ok.
I don't see any build errors either.

Are we mixing up things here maybe?

Helge

^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Andy Shevchenko @ 2026-03-19  9:08 UTC (permalink / raw)
  To: Helge Deller
  Cc: Jason Yan, Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <c717b7b6-ffb6-47f9-b345-de0eddcfe7a4@gmx.de>

On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
> On 3/19/26 09:21, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> > > 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > > > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:

...

> > > > Helge, can we revert this change and start over, please? (I can also send
> > > > revert if you think it's a better way)
> 
> Andy, all points you make against removing relevant code is absolutely right.
> 
> But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
> set but not used variable") I have to agree with Jason, that the patch is ok.
> I don't see any build errors either.

Just run on today's Linux Next (since the driver has not been changed in that
sense for a few years, the very same issue is present for a long time):

drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
  412 |                                 unsigned int mnp;
      |                                              ^
1 error generated.

> Are we mixing up things here maybe?

...

FWIW, I dove to the history of the driver.

So, the code, that was guarded by #if 0 was introduced in the original commit
213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded in the
commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on G450/G550.
Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC from older
DAC1064 handling. Modify PLL setting when both CRTCs use same pixel clocks.").

Even without that guard the modern compilers may see that the pixel_vco wasn't
ever used and seems a leftover after some debug or review made 25 years ago.

The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem to have
any side effects either than some unneeded CPU processing during runtime. I
agree that's unlikely that timeout (or heating up the CPU) has any effect on
the HW (GPU/display) functionality.

So, since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
and set but not used variable") the 'mnp' became unused, but eliminating that
code might have side-effects. The question here is what should we do with mnp?
The easiest way out is just mark it with __maybe_unused which will shut the
compiler up and won't change any possible IO flow.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* [syzbot] Monthly fbdev report (Mar 2026)
From: syzbot @ 2026-03-19 10:23 UTC (permalink / raw)
  To: deller, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs

Hello fbdev maintainers/developers,

This is a 31-day syzbot report for the fbdev subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/fbdev

During the period, 0 new issues were detected and 0 were fixed.
In total, 5 issues are still open and 29 have already been fixed.

Some of the still happening issues:

Ref Crashes Repro Title
<1> 2249    Yes   KASAN: vmalloc-out-of-bounds Write in imageblit (6)
                  https://syzkaller.appspot.com/bug?extid=5a40432dfe8f86ee657a
<2> 1510    Yes   KASAN: slab-out-of-bounds Read in fbcon_prepare_logo
                  https://syzkaller.appspot.com/bug?extid=0c815b25cdb3678e7083
<3> 195     No    KASAN: vmalloc-out-of-bounds Write in fillrect
                  https://syzkaller.appspot.com/bug?extid=7a63ce155648954e749b

---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Gary Guo @ 2026-03-19 11:39 UTC (permalink / raw)
  To: Joel Fernandes, linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Alex Gaynor, Danilo Krummrich, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260317201710.934932-2-joelagnelf@nvidia.com>

On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
> Add a new module `kernel::interop::list` for working with C's doubly
> circular linked lists. Provide low-level iteration over list nodes.
>
> Typed iteration over actual items is provided with a `clist_create`
> macro to assist in creation of the `CList` type.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Gary Guo <gary@garyguo.net>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  MAINTAINERS                 |   8 +
>  rust/helpers/helpers.c      |   1 +
>  rust/helpers/list.c         |  17 ++
>  rust/kernel/interop.rs      |   9 +
>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs          |   2 +
>  6 files changed, 379 insertions(+)
>  create mode 100644 rust/helpers/list.c
>  create mode 100644 rust/kernel/interop.rs
>  create mode 100644 rust/kernel/interop/list.rs
>
> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
> +///
> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
> +/// linked via the `$field` field in the underlying C struct `$c_type`.
> +///
> +/// # Arguments
> +///
> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
> +/// - `$rust_type`: Each item's rust wrapper type.
> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
> +/// - `$field`: The name of the `list_head` field within the C struct.
> +///
> +/// # Safety
> +///
> +/// The caller must ensure:
> +///
> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
> +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
> +#[macro_export]
> +macro_rules! clist_create {
> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
> +        // Compile-time check that field path is a `list_head`.
> +        // SAFETY: `p` is a valid pointer to `$c_type`.
> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
> +            |p| unsafe { &raw const (*p).$($field).+ };

Actually, this check is insufficient, you should create a reference instead
(just in case people put this inside `repr(packed)`.

This could be something like

    let _ = |p: &$c_type| { _ = &p.$($field).+ }

?

> +
> +        // Calculate offset and create `CList`.
> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }

Given that this is unsafe, I am not sure why the macro should have unsafe
keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
user write unsafe.

The offset could probably benefit from similar strategies we used for other
embedded data structure like Rust list and work items. Boqun has a series
unifying these and clist could probably use it, too. (As a follow-up)

Best,
Gary

> +    }};
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index d93292d47420..bdcf632050ee 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -29,6 +29,7 @@
>  #![feature(lint_reasons)]
>  //
>  // Stable since Rust 1.82.0.
> +#![feature(offset_of_nested)]
>  #![feature(raw_ref_op)]
>  //
>  // Stable since Rust 1.83.0.
> @@ -107,6 +108,7 @@
>  #[doc(hidden)]
>  pub mod impl_flags;
>  pub mod init;
> +pub mod interop;
>  pub mod io;
>  pub mod ioctl;
>  pub mod iommu;
G

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Alexandre Courbot @ 2026-03-19 11:59 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Miguel Ojeda, Alejandra González, Alice Ryhl, linux-kernel,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Alex Gaynor,
	Danilo Krummrich, Dave Airlie, David Airlie, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Daniel Almeida,
	Koen Koning, Nikola Djukic, Philipp Stanner, Elle Rhumsaa,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, John Hubbard,
	Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, alexeyi, Eliot Courtney,
	dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <20260318192446.GA591541@joelbox2>

On Thu Mar 19, 2026 at 4:24 AM JST, Joel Fernandes wrote:
> On Wed, Mar 18, 2026 at 07:57:14PM +0100, Miguel Ojeda wrote:
>> On Wed, Mar 18, 2026 at 7:31 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>> >
>> > Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
>> > here, instead of // SAFETY:
>> > https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/
>> 
>> So, to clarify, I suggested it as a temporary thing we could do if we
>> want to use that "fake `unsafe` block in macro matcher" pattern more
>> and more.
>> 
>> i.e. if we plan to use the pattern more, then I am happy to ask
>> upstream if it would make sense for Clippy to recognize it (or perhaps
>> it is just a false negative instead of a false positive, given
>> `impl_device_context_deref`), so that we don't need a hacked safety
>> tag (Cc'ing Alejandra).
>> 
>> But if we could put it outside, then we wouldn't need any of that.
>> Unsafe macros support could help perhaps here, which I have had it in
>> our wishlist too (https://github.com/Rust-for-Linux/linux/issues/354),
>> but I guess the fake block could still be useful to make only certain
>> macro arms unsafe? (Perhaps Rust could allow `unsafe` just at the
>> start of each arm for that...).
>
> Even if I reworked the macro to be outisde, it doesn't work as below, still
> need the 'disabled' comment on the macro's generate unsafe { } block below.
>
> If we don't want the SAFETY*: hack, we could do the following.
>
> Perhaps, we can file the github bug and also do the below. Once the
> github bug is fixed, we could remove the 'disable lint' below.
>
> Thoughts? 
>
> ---8<-----------------------
>
> diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
> index 495497f0405e..dfa2e1490202 100644
> --- a/rust/kernel/interop/list.rs
> +++ b/rust/kernel/interop/list.rs
> @@ -73,7 +73,7 @@
>  //!
>  //!
>  //! // Create typed [`CList`] from sentinel head.
> -//! // SAFETY*: `head` is valid and initialized, items are `SampleItemC` with
> +//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>  //! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
>  //! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
>  //!
> @@ -328,17 +328,19 @@ impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
>  /// Refer to the examples in the [`crate::interop::list`] module documentation.
>  #[macro_export]
>  macro_rules! clist_create {
> -    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => (
> +        // SAFETY: disable lint.
> +        unsafe { {{
>          // Compile-time check that field path is a `list_head`.
>          let _: fn(*const $c_type) -> *const $crate::bindings::list_head = |p| {
>              // SAFETY: `p` is a valid pointer to `$c_type`.
> -            unsafe { &raw const (*p).$($field).+ }
> +            &raw const (*p).$($field).+
>          };
>  
>          // Calculate offset and create `CList`.
>          const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>          // SAFETY: The caller of this macro is responsible for ensuring safety.
> -        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
> -    }};
> +        $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head)
> +    } }});
>  }
>  pub use clist_create;

I think I like this, it preserves the expected use of `SAFETY:` without
that confusing `*`. The unsafe blocks is a bit larger that it should, be
we are in a controlled environment.

Even after using the `SAFETY*:` I was still getting errors because the
in-macro SAFETY comment wasn't at the right place:

    warning: unsafe block missing a safety comment
      --> ../rust/kernel/interop/list.rs:335:17
        |
    335 |               |p| unsafe { &raw const (*p).$($field).+ };
        |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |
      ::: ../rust/kernel/gpu/buddy.rs:527:21
        |
    527 |           let clist = clist_create!(unsafe {
        |  _____________________-
    528 | |             head,
    529 | |             Block,
    530 | |             bindings::gpu_buddy_block,
    531 | |             __bindgen_anon_1.link
    532 | |         });
        | |__________- in this macro invocation

The diff below fixes that, but I believe your proposal should as well on
top of letting callers use the expected SAFETY statement.

--- a/rust/kernel/interop/list.rs
+++ b/rust/kernel/interop/list.rs
@@ -330,8 +330,8 @@ impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
 macro_rules! clist_create {
     (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
         // Compile-time check that field path is a `list_head`.
-        // SAFETY: `p` is a valid pointer to `$c_type`.
         let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
+            // SAFETY: `p` is a valid pointer to `$c_type`.
             |p| unsafe { &raw const (*p).$($field).+ };

         // Calculate offset and create `CList`.


^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Danilo Krummrich @ 2026-03-19 12:05 UTC (permalink / raw)
  To: Gary Guo
  Cc: Joel Fernandes, linux-kernel, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Alex Gaynor, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH6QAR1HHXRV.1Y7IZ22HC9FZ3@garyguo.net>

On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote:
> On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
>> Add a new module `kernel::interop::list` for working with C's doubly
>> circular linked lists. Provide low-level iteration over list nodes.
>>
>> Typed iteration over actual items is provided with a `clist_create`
>> macro to assist in creation of the `CList` type.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Gary Guo <gary@garyguo.net>
>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  MAINTAINERS                 |   8 +
>>  rust/helpers/helpers.c      |   1 +
>>  rust/helpers/list.c         |  17 ++
>>  rust/kernel/interop.rs      |   9 +
>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs          |   2 +
>>  6 files changed, 379 insertions(+)
>>  create mode 100644 rust/helpers/list.c
>>  create mode 100644 rust/kernel/interop.rs
>>  create mode 100644 rust/kernel/interop/list.rs
>>
>> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
>> +///
>> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
>> +/// linked via the `$field` field in the underlying C struct `$c_type`.
>> +///
>> +/// # Arguments
>> +///
>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>> +/// - `$rust_type`: Each item's rust wrapper type.
>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>> +/// - `$field`: The name of the `list_head` field within the C struct.
>> +///
>> +/// # Safety
>> +///
>> +/// The caller must ensure:
>> +///
>> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
>> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>> +///
>> +/// # Examples
>> +///
>> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
>> +#[macro_export]
>> +macro_rules! clist_create {
>> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
>> +        // Compile-time check that field path is a `list_head`.
>> +        // SAFETY: `p` is a valid pointer to `$c_type`.
>> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
>> +            |p| unsafe { &raw const (*p).$($field).+ };
>
> Actually, this check is insufficient, you should create a reference instead
> (just in case people put this inside `repr(packed)`.
>
> This could be something like
>
>     let _ = |p: &$c_type| { _ = &p.$($field).+ }
>
> ?
>
>> +
>> +        // Calculate offset and create `CList`.
>> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
>> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
>
> Given that this is unsafe, I am not sure why the macro should have unsafe
> keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
> user write unsafe.

Either you are proposing to not wrap unsafe code within unsafe {} within the
macro, such that the user is forced to write an unsafe {} around the macro, but
then they calls within the macro are not justified individually, or you propose
to let the user write an unsafe {} around the macro regardless of the inner
unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe
and nothing forces the user to actually wrap it in unsafe {}.

Is there a third option I'm not aware of? I.e. for the above reason
impl_device_context_deref!() was designed the same way.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n650

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Gary Guo @ 2026-03-19 12:21 UTC (permalink / raw)
  To: Danilo Krummrich, Gary Guo
  Cc: Joel Fernandes, linux-kernel, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Alex Gaynor, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH6QUO2T941E.2S1UP7EABOP42@kernel.org>

On Thu Mar 19, 2026 at 12:05 PM GMT, Danilo Krummrich wrote:
> On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote:
>> On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
>>> Add a new module `kernel::interop::list` for working with C's doubly
>>> circular linked lists. Provide low-level iteration over list nodes.
>>>
>>> Typed iteration over actual items is provided with a `clist_create`
>>> macro to assist in creation of the `CList` type.
>>>
>>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Acked-by: Gary Guo <gary@garyguo.net>
>>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> ---
>>>  MAINTAINERS                 |   8 +
>>>  rust/helpers/helpers.c      |   1 +
>>>  rust/helpers/list.c         |  17 ++
>>>  rust/kernel/interop.rs      |   9 +
>>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>>  rust/kernel/lib.rs          |   2 +
>>>  6 files changed, 379 insertions(+)
>>>  create mode 100644 rust/helpers/list.c
>>>  create mode 100644 rust/kernel/interop.rs
>>>  create mode 100644 rust/kernel/interop/list.rs
>>>
>>> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
>>> +///
>>> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
>>> +/// linked via the `$field` field in the underlying C struct `$c_type`.
>>> +///
>>> +/// # Arguments
>>> +///
>>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>>> +/// - `$rust_type`: Each item's rust wrapper type.
>>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>>> +/// - `$field`: The name of the `list_head` field within the C struct.
>>> +///
>>> +/// # Safety
>>> +///
>>> +/// The caller must ensure:
>>> +///
>>> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
>>> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
>>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
>>> +#[macro_export]
>>> +macro_rules! clist_create {
>>> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
>>> +        // Compile-time check that field path is a `list_head`.
>>> +        // SAFETY: `p` is a valid pointer to `$c_type`.
>>> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
>>> +            |p| unsafe { &raw const (*p).$($field).+ };
>>
>> Actually, this check is insufficient, you should create a reference instead
>> (just in case people put this inside `repr(packed)`.
>>
>> This could be something like
>>
>>     let _ = |p: &$c_type| { _ = &p.$($field).+ }
>>
>> ?
>>
>>> +
>>> +        // Calculate offset and create `CList`.
>>> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>>> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
>>> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
>>
>> Given that this is unsafe, I am not sure why the macro should have unsafe
>> keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
>> user write unsafe.
>
> Either you are proposing to not wrap unsafe code within unsafe {} within the
> macro, such that the user is forced to write an unsafe {} around the macro, but
> then they calls within the macro are not justified individually, or you propose
> to let the user write an unsafe {} around the macro regardless of the inner
> unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe
> and nothing forces the user to actually wrap it in unsafe {}.

The former.

"The caller of this macro is responsible for ensuring safety" justification is
not really useful here IMO.

If there're cases where we do want to justify unsafe code that's not immediately
deferring to the user inside the macro, we could use the SAFETY* trick proposed
in the thread, without writing an actual `unsafe {}` block.

>
> Is there a third option I'm not aware of? I.e. for the above reason
> impl_device_context_deref!() was designed the same way.

impl_device_context_deref!() expands to an item, so the user couldn't put an
`unsafe {}` on the outside. This macro expands to an expression, so users can
add `unsafe` themselves.

Best,
Gary

>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n650


^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Helge Deller @ 2026-03-19 12:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jason Yan, Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <abu9DKMN660yd3Sd@ashevche-desk.local>

On 3/19/26 10:08, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
>> On 3/19/26 09:21, Andy Shevchenko wrote:
>>> On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
>>>> 在 2026/3/19 15:38, Andy Shevchenko 写道:
>>>>> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> 
> ...
> 
>>>>> Helge, can we revert this change and start over, please? (I can also send
>>>>> revert if you think it's a better way)
>>
>> Andy, all points you make against removing relevant code is absolutely right.
>>
>> But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
>> set but not used variable") I have to agree with Jason, that the patch is ok.
>> I don't see any build errors either.
> 
> Just run on today's Linux Next (since the driver has not been changed in that
> sense for a few years, the very same issue is present for a long time):
> 
> drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
>    412 |                                 unsigned int mnp;
>        |                                              ^
> 1 error generated.

Ok.

  
>> Are we mixing up things here maybe?
> 
> ...
> 
> FWIW, I dove to the history of the driver.
> 
> So, the code, that was guarded by #if 0 was introduced in the original commit
> 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded in the
> commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on G450/G550.
> Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC from older
> DAC1064 handling. Modify PLL setting when both CRTCs use same pixel clocks.").
> 
> Even without that guard the modern compilers may see that the pixel_vco wasn't
> ever used and seems a leftover after some debug or review made 25 years ago.
> 
> The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem to have
> any side effects either than some unneeded CPU processing during runtime. I
> agree that's unlikely that timeout (or heating up the CPU) has any effect on
> the HW (GPU/display) functionality.
> 
> So, since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
> and set but not used variable") the 'mnp' became unused, but eliminating that
> code might have side-effects. The question here is what should we do with mnp?
> The easiest way out is just mark it with __maybe_unused which will shut the
> compiler up and won't change any possible IO flow.

Yes, I think that's what I'd prefer.
Do you mind sending a patch?

Helge

^ permalink raw reply

* Re: [PATCH v13 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Alexandre Courbot @ 2026-03-19 12:49 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
	Koen Koning, dri-devel, rust-for-linux, Nikola Djukic,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jonathan Corbet, Alex Deucher,
	Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller, Alex Gaynor,
	Boqun Feng, John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	Philipp Stanner, Elle Rhumsaa, alexeyi, Eliot Courtney, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260317220323.1909618-2-joelagnelf@nvidia.com>

Hi Joel,

On Wed Mar 18, 2026 at 7:03 AM JST, Joel Fernandes wrote:
> Add safe Rust abstractions over the Linux kernel's GPU buddy
> allocator for physical memory management. The GPU buddy allocator
> implements a binary buddy system useful for GPU physical memory
> allocation. nova-core will use it for physical memory allocation.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

A few things came to mind when re-reading this again. I believe these
will be my final comments on this patch though (famous last words).

<snip>
> +//! # Examples
> +//!
> +//! Create a buddy allocator and perform a basic range allocation:
> +//!
> +//! ```
> +//! use kernel::{
> +//!     gpu::buddy::{
> +//!         GpuBuddy,
> +//!         GpuBuddyAllocFlags,
> +//!         GpuBuddyAllocMode,
> +//!         GpuBuddyParams, //
> +//!     },
> +//!     prelude::*,
> +//!     ptr::Alignment,
> +//!     sizes::*, //
> +//! };
> +//!
> +//! // Create a 1GB buddy allocator with 4KB minimum chunk size.
> +//! let buddy = GpuBuddy::new(GpuBuddyParams {
> +//!     base_offset: 0,
> +//!     physical_memory_size: SZ_1G as u64,
> +//!     chunk_size: Alignment::new::<SZ_4K>(),
> +//! })?;
> +//!
> +//! assert_eq!(buddy.size(), SZ_1G as u64);
> +//! assert_eq!(buddy.chunk_size(), Alignment::new::<SZ_4K>());

Note that you can also use

  assert_eq!(buddy.chunk_size().as_usize(), SZ_4K);

To avoid the `Alignment` constructor.

<snip>
> +impl GpuBuddyAllocMode {
> +    // Returns the C flags corresponding to the allocation mode.
> +    fn into_flags(self) -> usize {
> +        match self {
> +            Self::Simple => 0,
> +            Self::Range { .. } => bindings::GPU_BUDDY_RANGE_ALLOCATION,
> +            Self::TopDown => bindings::GPU_BUDDY_TOPDOWN_ALLOCATION,
> +        }
> +    }
> +
> +    // Extracts the range start/end, defaulting to (0, 0) for non-range modes.

Let's use `(0, 0)` so they are properly formatted (I know it's not a
doccomment, but the convention also applies to regular comments).

> +    fn range(self) -> (u64, u64) {
> +        match self {
> +            Self::Range { start, end } => (start, end),
> +            _ => (0, 0),
> +        }
> +    }
> +}
> +
> +crate::impl_flags!(
> +    /// Modifier flags for GPU buddy allocation.
> +    ///
> +    /// These flags can be combined with any [`GpuBuddyAllocMode`] to control
> +    /// additional allocation behavior.
> +    #[derive(Clone, Copy, Default, PartialEq, Eq)]
> +    pub struct GpuBuddyAllocFlags(u32);

I've realized a bit late that this should actually be
`GpuBuddyAllocFlags(usize)`.

The values are defined in the bindings as `usize`, and we convert them
to `u32`, only to convert them back into `usize` in `alloc_blocks`. I
know it goes against the idea that flags should not have a size
dependent on the architecture, but in this case it's just a consequence
of the C API not doing it - and in the end we have to conform, so there
is no point in resisting. Actually, `GpuBuddyAllocMode::into_flags`
already return a `usize`, so we're already halfway there.

Just going with the flow and using `usize` removes quite a few `as` in
the code. Ideally we would fix the C API and switch back to `u32` in the
near future but for now that's the best course of action imho.

I've checked whether it worked, and it does - here is a diff for reference:

https://github.com/Gnurou/linux/commit/2e1bfc2d8e1f93a76343c7c563b1f4b85a69ab8b

<snip>
> +    /// Get the base offset for allocations.
> +    pub fn base_offset(&self) -> u64 {
> +        self.0.params.base_offset
> +    }
> +
> +    /// Get the chunk size (minimum allocation unit).
> +    pub fn chunk_size(&self) -> Alignment {
> +        self.0.params.chunk_size
> +    }
> +
> +    /// Get the total managed size.
> +    pub fn size(&self) -> u64 {
> +        self.0.params.physical_memory_size
> +    }
> +
> +    /// Get the available (free) memory in bytes.
> +    pub fn free_memory(&self) -> u64 {

This name is a bit confusing as it can be interpreted as this method
frees memory, whereas it doesn't free anything, and doesn't even deal
with memory (but an address space that may or may not represent memory).

In the C `struct gpu_buddy`, the member representing the chunk size is
named `chunk_size`, and the total size `size`, making the two methods
above this one adopt the same name (by a happy coincidence maybe :)).

Let's do the same here - since we are querying `avail`, this method can
just be called `avail` to align with the C API.

In the same spirit, we should rename
`GpuBuddyParams::physical_memory_size` into just `size` because that's
the name of the corresponding field in `struct gpu_buddy` and again, we
are not limited to managing physical memory with this allocator.

> +        let guard = self.0.lock();
> +
> +        // SAFETY: Per the type invariant, `inner` contains an initialized allocator.
> +        // `guard` provides exclusive access.
> +        unsafe { (*guard.as_raw()).avail }
> +    }
> +
> +    /// Allocate blocks from the buddy allocator.
> +    ///
> +    /// Returns a pin-initializer for [`AllocatedBlocks`].
> +    pub fn alloc_blocks(
> +        &self,
> +        mode: GpuBuddyAllocMode,
> +        size: u64,
> +        min_block_size: Alignment,
> +        flags: impl Into<GpuBuddyAllocFlags>,
> +    ) -> impl PinInit<AllocatedBlocks, Error> {
> +        let buddy_arc = Arc::clone(&self.0);
> +        let (start, end) = mode.range();
> +        let mode_flags = mode.into_flags();
> +        let modifier_flags = u32::from(flags.into()) as usize;
> +
> +        // Create pin-initializer that initializes list and allocates blocks.
> +        try_pin_init!(AllocatedBlocks {
> +            buddy: buddy_arc,
> +            list <- CListHead::new(),
> +            _: {
> +                // Reject zero-sized or inverted ranges.
> +                if let GpuBuddyAllocMode::Range { start, end } = mode {
> +                    if end <= start {
> +                        Err::<(), Error>(EINVAL)?;
> +                    }
> +                }

Ah, indeed we want to disallow decreasing ranges. Actually, why not
prevent them from even being expressed by using an actual Rust `Range`?

This lets you turn this test into an `is_empty()` and removes 10 LoCs
overall. You lose the ability to copy `GpuBuddyAllocMode`, but we don't
need it in the first place.

Here is a diff showing what it looks like, feel free to pick it:

https://github.com/Gnurou/linux/commit/7f9348f6a64d0fbec7ddf99b78ca727a1ac1cd06


^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Danilo Krummrich @ 2026-03-19 12:51 UTC (permalink / raw)
  To: Gary Guo
  Cc: Joel Fernandes, linux-kernel, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Alex Gaynor, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH6R6GB10S07.AG2EY39F0P85@garyguo.net>

On Thu Mar 19, 2026 at 1:21 PM CET, Gary Guo wrote:
> If there're cases where we do want to justify unsafe code that's not immediately
> deferring to the user inside the macro, we could use the SAFETY* trick proposed
> in the thread, without writing an actual `unsafe {}` block.

Works for me -- if this is what we want to do in such cases, we should probably
document it somewhere in the coding guidelines.

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-19 16:56 UTC (permalink / raw)
  To: Gary Guo, Danilo Krummrich
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Alex Gaynor, Dave Airlie, David Airlie, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Daniel Almeida,
	Koen Koning, Nikola Djukic, Alexandre Courbot, Philipp Stanner,
	Elle Rhumsaa, Jonathan Corbet, Alex Deucher, Christian König,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Huang Rui, Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, John Hubbard,
	Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, alexeyi, Eliot Courtney,
	dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <DH6R6GB10S07.AG2EY39F0P85@garyguo.net>



On 3/19/2026 8:21 AM, Gary Guo wrote:
> On Thu Mar 19, 2026 at 12:05 PM GMT, Danilo Krummrich wrote:
>> On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote:
>>> On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
>>>> Add a new module `kernel::interop::list` for working with C's doubly
>>>> circular linked lists. Provide low-level iteration over list nodes.
>>>>
>>>> Typed iteration over actual items is provided with a `clist_create`
>>>> macro to assist in creation of the `CList` type.
>>>>
>>>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> Acked-by: Gary Guo <gary@garyguo.net>
>>>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>>> ---
>>>>  MAINTAINERS                 |   8 +
>>>>  rust/helpers/helpers.c      |   1 +
>>>>  rust/helpers/list.c         |  17 ++
>>>>  rust/kernel/interop.rs      |   9 +
>>>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>>>  rust/kernel/lib.rs          |   2 +
>>>>  6 files changed, 379 insertions(+)
>>>>  create mode 100644 rust/helpers/list.c
>>>>  create mode 100644 rust/kernel/interop.rs
>>>>  create mode 100644 rust/kernel/interop/list.rs
>>>>
>>>> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
>>>> +///
>>>> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
>>>> +/// linked via the `$field` field in the underlying C struct `$c_type`.
>>>> +///
>>>> +/// # Arguments
>>>> +///
>>>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>>>> +/// - `$rust_type`: Each item's rust wrapper type.
>>>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>>>> +/// - `$field`: The name of the `list_head` field within the C struct.
>>>> +///
>>>> +/// # Safety
>>>> +///
>>>> +/// The caller must ensure:
>>>> +///
>>>> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
>>>> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
>>>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>>>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>>>> +///
>>>> +/// # Examples
>>>> +///
>>>> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
>>>> +#[macro_export]
>>>> +macro_rules! clist_create {
>>>> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
>>>> +        // Compile-time check that field path is a `list_head`.
>>>> +        // SAFETY: `p` is a valid pointer to `$c_type`.
>>>> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
>>>> +            |p| unsafe { &raw const (*p).$($field).+ };
>>>
>>> Actually, this check is insufficient, you should create a reference instead
>>> (just in case people put this inside `repr(packed)`.
>>>
>>> This could be something like
>>>
>>>     let _ = |p: &$c_type| { _ = &p.$($field).+ }
>>>
>>> ?
>>>
>>>> +
>>>> +        // Calculate offset and create `CList`.
>>>> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>>>> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
>>>> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
>>>
>>> Given that this is unsafe, I am not sure why the macro should have unsafe
>>> keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
>>> user write unsafe.
>>
>> Either you are proposing to not wrap unsafe code within unsafe {} within the
>> macro, such that the user is forced to write an unsafe {} around the macro, but
>> then they calls within the macro are not justified individually, or you propose
>> to let the user write an unsafe {} around the macro regardless of the inner
>> unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe
>> and nothing forces the user to actually wrap it in unsafe {}.
> 
> The former.
> 
> "The caller of this macro is responsible for ensuring safety" justification is
> not really useful here IMO.
> 
> If there're cases where we do want to justify unsafe code that's not immediately
> deferring to the user inside the macro, we could use the SAFETY* trick proposed
> in the thread, without writing an actual `unsafe {}` block.
> 
>>
>> Is there a third option I'm not aware of? I.e. for the above reason
>> impl_device_context_deref!() was designed the same way.
> 
> impl_device_context_deref!() expands to an item, so the user couldn't put an
> `unsafe {}` on the outside. This macro expands to an expression, so users can
> add `unsafe` themselves.
> 
I like Gary's idea. I will drop the unsafe { } blocks within the macro and
we can force the caller to clear the lint. That's the cleanest and most
reasonable IMO, instead of working around the linter.

Unless someone yells, this is what I'll do for the next iteration.

--
Joel Fernandes


^ permalink raw reply

* [PATCH v13.1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-19 21:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: Gary Guo, Danilo Krummrich, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Alex Gaynor, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev, Joel Fernandes

Add a new module `kernel::interop::list` for working with C's doubly
circular linked lists. Provide low-level iteration over list nodes.

Typed iteration over actual items is provided with a `clist_create`
macro to assist in creation of the `CList` type.

Cc: Nikola Djukic <ndjukic@nvidia.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Gary Guo <gary@garyguo.net>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
Just resending this with changes to clippy safety lint fix (link:
https://lore.kernel.org/all/4728e901-df27-4685-a21a-d33a84946558@nvidia.com/)

 MAINTAINERS                 |   8 +
 rust/helpers/helpers.c      |   1 +
 rust/helpers/list.c         |  17 ++
 rust/kernel/interop/list.rs | 341 ++++++++++++++++++++++++++++++++++++
 rust/kernel/interop/mod.rs  |   9 +
 rust/kernel/lib.rs          |   2 +
 6 files changed, 378 insertions(+)
 create mode 100644 rust/helpers/list.c
 create mode 100644 rust/kernel/interop/list.rs
 create mode 100644 rust/kernel/interop/mod.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 4bd6b538a51f..e847099efcc2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23251,6 +23251,14 @@ T:	git https://github.com/Rust-for-Linux/linux.git alloc-next
 F:	rust/kernel/alloc.rs
 F:	rust/kernel/alloc/
 
+RUST [INTEROP]
+M:	Joel Fernandes <joelagnelf@nvidia.com>
+M:	Alexandre Courbot <acourbot@nvidia.com>
+L:	rust-for-linux@vger.kernel.org
+S:	Maintained
+T:	git https://github.com/Rust-for-Linux/linux.git interop-next
+F:	rust/kernel/interop/
+
 RUST [NUM]
 M:	Alexandre Courbot <acourbot@nvidia.com>
 R:	Yury Norov <yury.norov@gmail.com>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index a3c42e51f00a..724fcb8240ac 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -35,6 +35,7 @@
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
+#include "list.c"
 #include "maple_tree.c"
 #include "mm.c"
 #include "mutex.c"
diff --git a/rust/helpers/list.c b/rust/helpers/list.c
new file mode 100644
index 000000000000..18095a5593c5
--- /dev/null
+++ b/rust/helpers/list.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Helpers for C circular doubly linked list implementation.
+ */
+
+#include <linux/list.h>
+
+__rust_helper void rust_helper_INIT_LIST_HEAD(struct list_head *list)
+{
+	INIT_LIST_HEAD(list);
+}
+
+__rust_helper void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
+{
+	list_add_tail(new, head);
+}
diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
new file mode 100644
index 000000000000..ae9692383462
--- /dev/null
+++ b/rust/kernel/interop/list.rs
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust interface for C doubly circular intrusive linked lists.
+//!
+//! This module provides Rust abstractions for iterating over C `list_head`-based
+//! linked lists. It should only be used for cases where C and Rust code share
+//! direct access to the same linked list through a C interop interface.
+//!
+//! Note: This *must not* be used by Rust components that just need a linked list
+//! primitive. Use [`kernel::list::List`] instead.
+//!
+//! # Examples
+//!
+//! ```
+//! use kernel::{
+//!     bindings,
+//!     interop::list::clist_create,
+//!     types::Opaque,
+//! };
+//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
+//! # // emulated here for doctests using the C bindings.
+//! # use core::mem::MaybeUninit;
+//! #
+//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
+//! # #[repr(C)]
+//! # pub struct SampleItemC {
+//! #     pub value: i32,
+//! #     pub link: bindings::list_head,
+//! # }
+//! #
+//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
+//! #
+//! # let head = head.as_mut_ptr();
+//! # // SAFETY: `head` and all the items are test objects allocated in this scope.
+//! # unsafe { bindings::INIT_LIST_HEAD(head) };
+//! #
+//! # let mut items = [
+//! #     MaybeUninit::<SampleItemC>::uninit(),
+//! #     MaybeUninit::<SampleItemC>::uninit(),
+//! #     MaybeUninit::<SampleItemC>::uninit(),
+//! # ];
+//! #
+//! # for (i, item) in items.iter_mut().enumerate() {
+//! #     let ptr = item.as_mut_ptr();
+//! #     // SAFETY: `ptr` points to a valid `MaybeUninit<SampleItemC>`.
+//! #     unsafe { (*ptr).value = i as i32 * 10 };
+//! #     // SAFETY: `&raw mut` creates a pointer valid for `INIT_LIST_HEAD`.
+//! #     unsafe { bindings::INIT_LIST_HEAD(&raw mut (*ptr).link) };
+//! #     // SAFETY: `link` was just initialized and `head` is a valid list head.
+//! #     unsafe { bindings::list_add_tail(&mut (*ptr).link, head) };
+//! # }
+//!
+//! //
+//! /// Rust wrapper for the C struct.
+//! ///
+//! /// The list item struct in this example is defined in C code as:
+//! ///
+//! /// ```c
+//! /// struct SampleItemC {
+//! ///     int value;
+//! ///     struct list_head link;
+//! /// };
+//! /// ```
+//! #[repr(transparent)]
+//! pub struct Item(Opaque<SampleItemC>);
+//!
+//! impl Item {
+//!     pub fn value(&self) -> i32 {
+//!         // SAFETY: `Item` has same layout as `SampleItemC`.
+//!         unsafe { (*self.0.get()).value }
+//!     }
+//! }
+//!
+//!
+//! // Create typed [`CList`] from sentinel head.
+//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
+//! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
+//! let list = unsafe { clist_create!(head, Item, SampleItemC, link) };
+//!
+//! // Iterate directly over typed items.
+//! let mut found_0 = false;
+//! let mut found_10 = false;
+//! let mut found_20 = false;
+//!
+//! for item in list.iter() {
+//!     let val = item.value();
+//!     if val == 0 { found_0 = true; }
+//!     if val == 10 { found_10 = true; }
+//!     if val == 20 { found_20 = true; }
+//! }
+//!
+//! assert!(found_0 && found_10 && found_20);
+//! ```
+
+use core::{
+    iter::FusedIterator,
+    marker::PhantomData, //
+};
+
+use crate::{
+    bindings,
+    types::Opaque, //
+};
+
+use pin_init::{
+    pin_data,
+    pin_init,
+    PinInit, //
+};
+
+/// FFI wrapper for a C `list_head` object used in intrusive linked lists.
+///
+/// # Invariants
+///
+/// - The underlying `list_head` is initialized with valid non-`NULL` `next`/`prev` pointers.
+#[pin_data]
+#[repr(transparent)]
+pub struct CListHead {
+    #[pin]
+    inner: Opaque<bindings::list_head>,
+}
+
+impl CListHead {
+    /// Create a `&CListHead` reference from a raw `list_head` pointer.
+    ///
+    /// # Safety
+    ///
+    /// - `ptr` must be a valid pointer to an initialized `list_head` (e.g. via
+    ///   `INIT_LIST_HEAD()`), with valid non-`NULL` `next`/`prev` pointers.
+    /// - `ptr` must remain valid for the lifetime `'a`.
+    /// - The list and all linked `list_head` nodes must not be modified from
+    ///   anywhere for the lifetime `'a`, unless done so via any [`CListHead`] APIs.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
+        // SAFETY:
+        // - `CListHead` has same layout as `list_head`.
+        // - `ptr` is valid and unmodified for `'a` per caller guarantees.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Get the raw `list_head` pointer.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::list_head {
+        self.inner.get()
+    }
+
+    /// Get the next [`CListHead`] in the list.
+    #[inline]
+    pub fn next(&self) -> &Self {
+        let raw = self.as_raw();
+        // SAFETY:
+        // - `self.as_raw()` is valid and initialized per type invariants.
+        // - The `next` pointer is valid and non-`NULL` per type invariants
+        //   (initialized via `INIT_LIST_HEAD()` or equivalent).
+        unsafe { Self::from_raw((*raw).next) }
+    }
+
+    /// Check if this node is linked in a list (not isolated).
+    #[inline]
+    pub fn is_linked(&self) -> bool {
+        let raw = self.as_raw();
+        // SAFETY: `self.as_raw()` is valid per type invariants.
+        unsafe { (*raw).next != raw && (*raw).prev != raw }
+    }
+
+    /// Pin-initializer that initializes the list head.
+    pub fn new() -> impl PinInit<Self> {
+        pin_init!(Self {
+            // SAFETY: `INIT_LIST_HEAD` initializes `slot` to a valid empty list.
+            inner <- Opaque::ffi_init(|slot| unsafe { bindings::INIT_LIST_HEAD(slot) }),
+        })
+    }
+}
+
+// SAFETY: `list_head` contains no thread-bound state; it only holds
+// `next`/`prev` pointers.
+unsafe impl Send for CListHead {}
+
+// SAFETY: `CListHead` can be shared among threads as modifications are
+// not allowed at the moment.
+unsafe impl Sync for CListHead {}
+
+impl PartialEq for CListHead {
+    #[inline]
+    fn eq(&self, other: &Self) -> bool {
+        core::ptr::eq(self, other)
+    }
+}
+
+impl Eq for CListHead {}
+
+/// Low-level iterator over `list_head` nodes.
+///
+/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
+/// perform conversion of returned [`CListHead`] to an item (using [`container_of`] or similar).
+///
+/// # Invariants
+///
+/// `current` and `sentinel` are valid references into an initialized linked list.
+struct CListHeadIter<'a> {
+    /// Current position in the list.
+    current: &'a CListHead,
+    /// The sentinel head (used to detect end of iteration).
+    sentinel: &'a CListHead,
+}
+
+impl<'a> Iterator for CListHeadIter<'a> {
+    type Item = &'a CListHead;
+
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        // Check if we've reached the sentinel (end of list).
+        if self.current == self.sentinel {
+            return None;
+        }
+
+        let item = self.current;
+        self.current = item.next();
+        Some(item)
+    }
+}
+
+impl<'a> FusedIterator for CListHeadIter<'a> {}
+
+/// A typed C linked list with a sentinel head intended for FFI use-cases where
+/// C subsystem manages a linked list that Rust code needs to read. Generally
+/// required only for special cases.
+///
+/// A sentinel head [`CListHead`] represents the entire linked list and can be used
+/// for iteration over items of type `T`, it is not associated with a specific item.
+///
+/// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
+/// the struct that `T` wraps.
+///
+/// # Invariants
+///
+/// - The sentinel [`CListHead`] has valid non-`NULL` `next`/`prev` pointers.
+/// - `OFFSET` is the byte offset of the `list_head` field within the struct that `T` wraps.
+/// - All the list's `list_head` nodes have valid non-`NULL` `next`/`prev` pointers.
+#[repr(transparent)]
+pub struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
+
+impl<T, const OFFSET: usize> CList<T, OFFSET> {
+    /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
+    ///
+    /// # Safety
+    ///
+    /// - `ptr` must be a valid pointer to an initialized sentinel `list_head` (e.g. via
+    ///   `INIT_LIST_HEAD()`), with valid non-`NULL` `next`/`prev` pointers.
+    /// - `ptr` must remain valid for the lifetime `'a`.
+    /// - The list and all linked nodes must not be concurrently modified for the lifetime `'a`.
+    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
+    /// - `T` must be `#[repr(transparent)]` over the C struct.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
+        // SAFETY:
+        // - `CList` has same layout as `CListHead` due to `#[repr(transparent)]`.
+        // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Check if the list is empty.
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        !self.0.is_linked()
+    }
+
+    /// Create an iterator over typed items.
+    #[inline]
+    pub fn iter(&self) -> CListIter<'_, T, OFFSET> {
+        let head = &self.0;
+        CListIter {
+            head_iter: CListHeadIter {
+                current: head.next(),
+                sentinel: head,
+            },
+            _phantom: PhantomData,
+        }
+    }
+}
+
+/// High-level iterator over typed list items.
+pub struct CListIter<'a, T, const OFFSET: usize> {
+    head_iter: CListHeadIter<'a>,
+    _phantom: PhantomData<&'a T>,
+}
+
+impl<'a, T, const OFFSET: usize> Iterator for CListIter<'a, T, OFFSET> {
+    type Item = &'a T;
+
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        let head = self.head_iter.next()?;
+
+        // Convert to item using `OFFSET`.
+        //
+        // SAFETY: The pointer calculation is valid because `OFFSET` is derived
+        // from `offset_of!` per type invariants.
+        Some(unsafe { &*head.as_raw().byte_sub(OFFSET).cast::<T>() })
+    }
+}
+
+impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
+
+/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
+///
+/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
+/// linked via the `$field` field in the underlying C struct `$c_type`.
+///
+/// # Arguments
+///
+/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
+/// - `$rust_type`: Each item's rust wrapper type.
+/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
+/// - `$field`: The name of the `list_head` field within the C struct.
+///
+/// # Safety
+///
+/// The caller must ensure:
+///
+/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
+///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
+/// - The list contains items of type `$c_type` linked via an embedded `$field`.
+/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
+///
+/// # Examples
+///
+/// Refer to the examples in the [`crate::interop::list`] module documentation.
+#[macro_export]
+macro_rules! clist_create {
+    ($head:expr, $rust_type:ty, $c_type:ty, $($field:tt).+) => {{
+        // Compile-time check that field path is a `list_head`.
+        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
+            |p| &raw const (*p).$($field).+;
+
+        // Calculate offset and create `CList`.
+        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
+        $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head)
+    }};
+}
+pub use clist_create;
diff --git a/rust/kernel/interop/mod.rs b/rust/kernel/interop/mod.rs
new file mode 100644
index 000000000000..b88140cf76dc
--- /dev/null
+++ b/rust/kernel/interop/mod.rs
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Infrastructure for interfacing Rust code with C kernel subsystems.
+//!
+//! This module is intended for low-level, unsafe Rust infrastructure code
+//! that interoperates between Rust and C. It is NOT for use directly in
+//! Rust drivers.
+
+pub mod list;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index d93292d47420..bdcf632050ee 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -29,6 +29,7 @@
 #![feature(lint_reasons)]
 //
 // Stable since Rust 1.82.0.
+#![feature(offset_of_nested)]
 #![feature(raw_ref_op)]
 //
 // Stable since Rust 1.83.0.
@@ -107,6 +108,7 @@
 #[doc(hidden)]
 pub mod impl_flags;
 pub mod init;
+pub mod interop;
 pub mod io;
 pub mod ioctl;
 pub mod iommu;
-- 
2.34.1


^ permalink raw reply related

* [PATCH v14 0/2] Rust GPU buddy allocator bindings
From: Joel Fernandes @ 2026-03-20  4:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
	dri-devel, rust-for-linux, Nikola Djukic, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Alex Gaynor, Boqun Feng,
	John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi,
	Eliot Courtney, joel, linux-doc, amd-gfx, intel-gfx, intel-xe,
	linux-fbdev, Joel Fernandes

This patch adds safe Rust abstractions over the Linux kernel's GPU buddy
allocator for physical memory management. The prerequisite infrastructure
patches (DRM buddy code movement and the uninitialized buddy fix) have been
absorbed into upstream -next, so this is now a set of standalone patches.

The series along with all dependencies, including clist and nova-core mm
patches, are available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (tag: buddy-bindings-v14-20260319)

Change log:

Changes from v13 to v14:
- Changed clist_create! call site to use explicit unsafe{} block.
- Various changes to change to Range type for ranges (Alex).
- Renamed GpuBuddyParams::physical_memory_size to size (Alex).
- Changed GpuBuddyAllocFlags inner type from u32 to usize (Alex).
- Renamed free_memory() to avail()  (Alex).
- Various nits (Alex).

Changes from v12 to v13:
- Split MAINTAINERS reviewer update into a separate patch (Danilo).
- Adjustments to use the Alignment type chunk size parameters (Alex).
- Fixed doctest assertion due to blocks landing on top of range.
- Changed chunk_size local vars to Alignment.
- Changed block size to u64 from usize.
- Renamed rust/kernel/gpu/mod.rs to rust/kernel/gpu.rs.
- Several other adjustments (Alex).

Changes from v11 to v12:
- Rebased on linux-next his is now a standalone single patch as dependencies
  are absorbed (but not clist is a prequisite)
- Redesigned allocation API (Alexandre Courbot) for better Rust ergonomics.
- Split single long example into 4 self-contained examples (Alexandre Courbot).
- Several safety and invariant comment changes (Danilo).
- MAINTAINERS changes (Arun, Mathew, Danilo, Dave).
- Fixed `#[cfg(CONFIG_GPU_BUDDY)]` to `#[cfg(CONFIG_GPU_BUDDY = "y")]` (Danilo Krummrich).
- Updated `ffi::clist::CListHead` to `interop::list::CListHead`.

Changes from v10 to v11:
- Dropped "rust: ffi: Convert pub use to pub mod and create ffi module" patch;
  the ffi module restructuring will go through a different path.
- Dropped "rust: clist: Add support to interface with C linked lists" patch;
  the clist module will be submitted separately.
- Dropped "nova-core: Kconfig: Sort select statements alphabetically" cosmetic
  patch.
- Patches 1-3 (DRM buddy movement and fix) are included as reference only;
  they are already being pulled into upstream via drm-misc-next.
- Removed clist patches as those can go in independently (Alice).
- Moved the Kconfig GPU_BUDDY selection patch to nova-core mm series to enable
  it when it is actually used.
- Various nits to comments, etc.

Changes from v9 to v10:
- Absorbed the DRM buddy code movement patches into this series as patches 1-2.
  Dave Airlie reworked these into two parts for better git history.
- Added "gpu: Fix uninitialized buddy for built-in drivers" fix by Koen Koning,
  using subsys_initcall instead of module_init to fix NULL pointer dereference
  when built-in drivers use the buddy allocator before initialization.
- Added "rust: ffi: Convert pub use to pub mod and create ffi module" to prepare
  the ffi module for hosting clist as a sub-module.
- Moved clist from rust/kernel/clist.rs to rust/kernel/ffi/.
- Added "nova-core: Kconfig: Sort select statements alphabetically" (Danilo).

Changes from v8 to v9:
- Updated nova-core Kconfig patch: addressed sorting of Kconfig options.
- Added Daniel Almeida's Reviewed-by tag to clist patch.
- Minor refinements to GPU buddy bindings.

Changes from v7 to v8:
- Added nova-core Kconfig patch to select GPU_BUDDY for VRAM allocation.
- Various changes suggested by Danilo Krummrich, Gary Guo, and Daniel Almeida.
- Added Acked-by: Gary Guo for clist patch.

Changes from v6 to v7:
- Major restructuring: split the large 26-patch v6 RFC series. v7 only contains
  the Rust infrastructure patches (clist + GPU buddy bindings), extracted from
  the full nova-core MM series. The nova-core MM patches follow separately.
- Rebased on linux-next.

Changes from v5 to v6:
- Rebased on drm-rust-kernel/drm-rust-next.
- Expanded from 6 to 26 patches with full nova-core MM infrastructure including
  page table walker, VMM, BAR1 user interface, TLB flush, and GpuMm manager.

Changes from v4 to v5:
- Added PRAMIN aperture support with documentation and self-tests.
- Improved buddy allocator bindings (fewer lines of code).
- Based on drm-rust-next instead of linux-next.

Changes from v3 to v4:
- Combined the clist and DRM buddy series into a single coherent series.
- Added DRM buddy allocator movement from drivers/gpu/drm/ up to drivers/gpu/,
  renaming API from drm_buddy to gpu_buddy.
- Added Rust bindings for the GPU buddy allocator.

Changes from v2 to v3:
- Squashed 3 clist patches into one due to inter-dependencies.
- Changed Clist to Clist<'a, T> using const generic offset (Alex Courbot).
- Simplified C helpers to only list_add_tail (Alex Courbot, John Hubbard).
- Added init_list_head() Rust function (Alex Courbot).
- Added FusedIterator, PartialEq/Eq impls.
- Added MAINTAINERS entry (Miguel Ojeda).

Changes from v1 (RFC) to v2:
- Dropped DRM buddy allocator patches; series focuses solely on clist module.
- Dropped sample modules, replaced with doctests.
- Added proper lifetime management similar to scatterlist.
- Split clist into 3 separate patches.

Link to v13: https://lore.kernel.org/all/20260317220323.1909618-1-joelagnelf@nvidia.com/


Joel Fernandes (2):
  rust: gpu: Add GPU buddy allocator bindings
  MAINTAINERS: gpu: buddy: Update reviewer

 MAINTAINERS                     |   8 +-
 rust/bindings/bindings_helper.h |  11 +
 rust/helpers/gpu.c              |  23 ++
 rust/helpers/helpers.c          |   1 +
 rust/kernel/gpu.rs              |   6 +
 rust/kernel/gpu/buddy.rs        | 613 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 7 files changed, 663 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/gpu.c
 create mode 100644 rust/kernel/gpu.rs
 create mode 100644 rust/kernel/gpu/buddy.rs

--
2.34.1


^ permalink raw reply

* [PATCH v14 2/2] MAINTAINERS: gpu: buddy: Update reviewer
From: Joel Fernandes @ 2026-03-20  4:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
	dri-devel, rust-for-linux, Nikola Djukic, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Alex Gaynor, Boqun Feng,
	John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi,
	Eliot Courtney, joel, linux-doc, amd-gfx, intel-gfx, intel-xe,
	linux-fbdev, Joel Fernandes
In-Reply-To: <20260320045711.43494-1-joelagnelf@nvidia.com>

Christian Koenig mentioned he'd like to step down from the reviewer
role for the GPU buddy allocator. Joel Fernandes is stepping in as
reviewer with agreement from Matthew Auld and Arun Pravin.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index cd9505d3be60..3353cbf98be1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8946,7 +8946,7 @@ F:	include/drm/ttm/
 GPU BUDDY ALLOCATOR
 M:	Matthew Auld <matthew.auld@intel.com>
 M:	Arun Pravin <arunpravin.paneerselvam@amd.com>
-R:	Christian Koenig <christian.koenig@amd.com>
+R:	Joel Fernandes <joelagnelf@nvidia.com>
 L:	dri-devel@lists.freedesktop.org
 S:	Maintained
 T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
-- 
2.34.1


^ permalink raw reply related

* [PATCH v14 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Joel Fernandes @ 2026-03-20  4:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
	dri-devel, rust-for-linux, Nikola Djukic, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Alex Gaynor, Boqun Feng,
	John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi,
	Eliot Courtney, joel, linux-doc, amd-gfx, intel-gfx, intel-xe,
	linux-fbdev, Joel Fernandes
In-Reply-To: <20260320045711.43494-1-joelagnelf@nvidia.com>

Add safe Rust abstractions over the Linux kernel's GPU buddy
allocator for physical memory management. The GPU buddy allocator
implements a binary buddy system useful for GPU physical memory
allocation. nova-core will use it for physical memory allocation.

Cc: Nikola Djukic <ndjukic@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 MAINTAINERS                     |   6 +
 rust/bindings/bindings_helper.h |  11 +
 rust/helpers/gpu.c              |  23 ++
 rust/helpers/helpers.c          |   1 +
 rust/kernel/gpu.rs              |   6 +
 rust/kernel/gpu/buddy.rs        | 613 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 7 files changed, 662 insertions(+)
 create mode 100644 rust/helpers/gpu.c
 create mode 100644 rust/kernel/gpu.rs
 create mode 100644 rust/kernel/gpu/buddy.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index e847099efcc2..cd9505d3be60 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8531,7 +8531,10 @@ T:	git https://gitlab.freedesktop.org/drm/rust/kernel.git
 F:	drivers/gpu/drm/nova/
 F:	drivers/gpu/drm/tyr/
 F:	drivers/gpu/nova-core/
+F:	rust/helpers/gpu.c
 F:	rust/kernel/drm/
+F:	rust/kernel/gpu.rs
+F:	rust/kernel/gpu/
 
 DRM DRIVERS FOR ALLWINNER A10
 M:	Chen-Yu Tsai <wens@kernel.org>
@@ -8952,6 +8955,9 @@ F:	drivers/gpu/drm/drm_buddy.c
 F:	drivers/gpu/tests/gpu_buddy_test.c
 F:	include/drm/drm_buddy.h
 F:	include/linux/gpu_buddy.h
+F:	rust/helpers/gpu.c
+F:	rust/kernel/gpu.rs
+F:	rust/kernel/gpu/
 
 DRM AUTOMATED TESTING
 M:	Helen Koike <helen.fornazier@gmail.com>
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 083cc44aa952..dbb765a9fdbd 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -29,6 +29,7 @@
 #include <linux/hrtimer_types.h>
 
 #include <linux/acpi.h>
+#include <linux/gpu_buddy.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
@@ -146,6 +147,16 @@ const vm_flags_t RUST_CONST_HELPER_VM_MIXEDMAP = VM_MIXEDMAP;
 const vm_flags_t RUST_CONST_HELPER_VM_HUGEPAGE = VM_HUGEPAGE;
 const vm_flags_t RUST_CONST_HELPER_VM_NOHUGEPAGE = VM_NOHUGEPAGE;
 
+#if IS_ENABLED(CONFIG_GPU_BUDDY)
+const unsigned long RUST_CONST_HELPER_GPU_BUDDY_RANGE_ALLOCATION = GPU_BUDDY_RANGE_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TOPDOWN_ALLOCATION = GPU_BUDDY_TOPDOWN_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CONTIGUOUS_ALLOCATION =
+								GPU_BUDDY_CONTIGUOUS_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEAR_ALLOCATION = GPU_BUDDY_CLEAR_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEARED = GPU_BUDDY_CLEARED;
+const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TRIM_DISABLE = GPU_BUDDY_TRIM_DISABLE;
+#endif
+
 #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
 #include "../../drivers/android/binder/rust_binder.h"
 #include "../../drivers/android/binder/rust_binder_events.h"
diff --git a/rust/helpers/gpu.c b/rust/helpers/gpu.c
new file mode 100644
index 000000000000..38b1a4e6bef8
--- /dev/null
+++ b/rust/helpers/gpu.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/gpu_buddy.h>
+
+#ifdef CONFIG_GPU_BUDDY
+
+__rust_helper u64 rust_helper_gpu_buddy_block_offset(const struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_offset(block);
+}
+
+__rust_helper unsigned int rust_helper_gpu_buddy_block_order(struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_order(block);
+}
+
+__rust_helper u64 rust_helper_gpu_buddy_block_size(struct gpu_buddy *mm,
+						   struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_size(mm, block);
+}
+
+#endif /* CONFIG_GPU_BUDDY */
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 724fcb8240ac..a53929ce52a3 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -32,6 +32,7 @@
 #include "err.c"
 #include "irq.c"
 #include "fs.c"
+#include "gpu.c"
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
diff --git a/rust/kernel/gpu.rs b/rust/kernel/gpu.rs
new file mode 100644
index 000000000000..1dc5d0c8c09d
--- /dev/null
+++ b/rust/kernel/gpu.rs
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! GPU subsystem abstractions.
+
+#[cfg(CONFIG_GPU_BUDDY = "y")]
+pub mod buddy;
diff --git a/rust/kernel/gpu/buddy.rs b/rust/kernel/gpu/buddy.rs
new file mode 100644
index 000000000000..fdf1fcc2dcee
--- /dev/null
+++ b/rust/kernel/gpu/buddy.rs
@@ -0,0 +1,613 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! GPU buddy allocator bindings.
+//!
+//! C header: [`include/linux/gpu_buddy.h`](srctree/include/linux/gpu_buddy.h)
+//!
+//! This module provides Rust abstractions over the Linux kernel's GPU buddy
+//! allocator, which implements a binary buddy memory allocator.
+//!
+//! The buddy allocator manages a contiguous address space and allocates blocks
+//! in power-of-two sizes, useful for GPU physical memory management.
+//!
+//! # Examples
+//!
+//! Create a buddy allocator and perform a basic range allocation:
+//!
+//! ```
+//! use kernel::{
+//!     gpu::buddy::{
+//!         GpuBuddy,
+//!         GpuBuddyAllocFlags,
+//!         GpuBuddyAllocMode,
+//!         GpuBuddyParams, //
+//!     },
+//!     prelude::*,
+//!     ptr::Alignment,
+//!     sizes::*, //
+//! };
+//!
+//! // Create a 1GB buddy allocator with 4KB minimum chunk size.
+//! let buddy = GpuBuddy::new(GpuBuddyParams {
+//!     base_offset: 0,
+//!     size: SZ_1G as u64,
+//!     chunk_size: Alignment::new::<SZ_4K>(),
+//! })?;
+//!
+//! assert_eq!(buddy.size(), SZ_1G as u64);
+//! assert_eq!(buddy.chunk_size(), Alignment::new::<SZ_4K>());
+//! let initial_free = buddy.avail();
+//!
+//! // Allocate 16MB. Block lands at the top of the address range.
+//! let allocated = KBox::pin_init(
+//!     buddy.alloc_blocks(
+//!         GpuBuddyAllocMode::Simple,
+//!         SZ_16M as u64,
+//!         Alignment::new::<SZ_16M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//! assert_eq!(buddy.avail(), initial_free - SZ_16M as u64);
+//!
+//! let block = allocated.iter().next().expect("expected one block");
+//! assert_eq!(block.offset(), (SZ_1G - SZ_16M) as u64);
+//! assert_eq!(block.order(), 12); // 2^12 pages = 16MB
+//! assert_eq!(block.size(), SZ_16M as u64);
+//! assert_eq!(allocated.iter().count(), 1);
+//!
+//! // Dropping the allocation returns the range to the buddy allocator.
+//! drop(allocated);
+//! assert_eq!(buddy.avail(), initial_free);
+//! # Ok::<(), Error>(())
+//! ```
+//!
+//! Top-down allocation allocates from the highest addresses:
+//!
+//! ```
+//! # use kernel::{
+//! #     gpu::buddy::{GpuBuddy, GpuBuddyAllocMode, GpuBuddyAllocFlags, GpuBuddyParams},
+//! #     prelude::*,
+//! #     ptr::Alignment,
+//! #     sizes::*, //
+//! # };
+//! # let buddy = GpuBuddy::new(GpuBuddyParams {
+//! #     base_offset: 0,
+//! #     size: SZ_1G as u64,
+//! #     chunk_size: Alignment::new::<SZ_4K>(),
+//! # })?;
+//! # let initial_free = buddy.avail();
+//! let topdown = KBox::pin_init(
+//!     buddy.alloc_blocks(
+//!         GpuBuddyAllocMode::TopDown,
+//!         SZ_16M as u64,
+//!         Alignment::new::<SZ_16M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//! assert_eq!(buddy.avail(), initial_free - SZ_16M as u64);
+//!
+//! let block = topdown.iter().next().expect("expected one block");
+//! assert_eq!(block.offset(), (SZ_1G - SZ_16M) as u64);
+//! assert_eq!(block.order(), 12);
+//! assert_eq!(block.size(), SZ_16M as u64);
+//!
+//! // Dropping the allocation returns the range to the buddy allocator.
+//! drop(topdown);
+//! assert_eq!(buddy.avail(), initial_free);
+//! # Ok::<(), Error>(())
+//! ```
+//!
+//! Non-contiguous allocation can fill fragmented memory by returning multiple
+//! blocks:
+//!
+//! ```
+//! # use kernel::{
+//! #     gpu::buddy::{
+//! #         GpuBuddy, GpuBuddyAllocFlags, GpuBuddyAllocMode, GpuBuddyParams,
+//! #     },
+//! #     prelude::*,
+//! #     ptr::Alignment,
+//! #     sizes::*, //
+//! # };
+//! # let buddy = GpuBuddy::new(GpuBuddyParams {
+//! #     base_offset: 0,
+//! #     size: SZ_1G as u64,
+//! #     chunk_size: Alignment::new::<SZ_4K>(),
+//! # })?;
+//! # let initial_free = buddy.avail();
+//! // Create fragmentation by allocating 4MB blocks at [0,4M) and [8M,12M).
+//! let frag1 = KBox::pin_init(
+//!     buddy.alloc_blocks(
+//!         GpuBuddyAllocMode::Range(0..SZ_4M as u64),
+//!         SZ_4M as u64,
+//!         Alignment::new::<SZ_4M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//! assert_eq!(buddy.avail(), initial_free - SZ_4M as u64);
+//!
+//! let frag2 = KBox::pin_init(
+//!     buddy.alloc_blocks(
+//!         GpuBuddyAllocMode::Range(SZ_8M as u64..(SZ_8M + SZ_4M) as u64),
+//!         SZ_4M as u64,
+//!         Alignment::new::<SZ_4M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//! assert_eq!(buddy.avail(), initial_free - SZ_8M as u64);
+//!
+//! // Allocate 8MB, this returns 2 blocks from the holes.
+//! let fragmented = KBox::pin_init(
+//!     buddy.alloc_blocks(
+//!         GpuBuddyAllocMode::Range(0..SZ_16M as u64),
+//!         SZ_8M as u64,
+//!         Alignment::new::<SZ_4M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//! assert_eq!(buddy.avail(), initial_free - SZ_16M as u64);
+//!
+//! let (mut count, mut total) = (0u32, 0u64);
+//! for block in fragmented.iter() {
+//!     assert_eq!(block.size(), SZ_4M as u64);
+//!     total += block.size();
+//!     count += 1;
+//! }
+//! assert_eq!(total, SZ_8M as u64);
+//! assert_eq!(count, 2);
+//! # Ok::<(), Error>(())
+//! ```
+//!
+//! Contiguous allocation fails when only fragmented space is available:
+//!
+//! ```
+//! # use kernel::{
+//! #     gpu::buddy::{
+//! #         GpuBuddy, GpuBuddyAllocFlag, GpuBuddyAllocFlags, GpuBuddyAllocMode, GpuBuddyParams,
+//! #     },
+//! #     prelude::*,
+//! #     ptr::Alignment,
+//! #     sizes::*, //
+//! # };
+//! // Create a small 16MB buddy allocator with fragmented memory.
+//! let small = GpuBuddy::new(GpuBuddyParams {
+//!     base_offset: 0,
+//!     size: SZ_16M as u64,
+//!     chunk_size: Alignment::new::<SZ_4K>(),
+//! })?;
+//!
+//! let _hole1 = KBox::pin_init(
+//!     small.alloc_blocks(
+//!         GpuBuddyAllocMode::Range(0..SZ_4M as u64),
+//!         SZ_4M as u64,
+//!         Alignment::new::<SZ_4M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//!
+//! let _hole2 = KBox::pin_init(
+//!     small.alloc_blocks(
+//!         GpuBuddyAllocMode::Range(SZ_8M as u64..(SZ_8M + SZ_4M) as u64),
+//!         SZ_4M as u64,
+//!         Alignment::new::<SZ_4M>(),
+//!         GpuBuddyAllocFlags::default(),
+//!     ),
+//!     GFP_KERNEL,
+//! )?;
+//!
+//! // 8MB contiguous should fail, only two non-contiguous 4MB holes exist.
+//! let result = KBox::pin_init(
+//!     small.alloc_blocks(
+//!         GpuBuddyAllocMode::Simple,
+//!         SZ_8M as u64,
+//!         Alignment::new::<SZ_4M>(),
+//!         GpuBuddyAllocFlag::Contiguous,
+//!     ),
+//!     GFP_KERNEL,
+//! );
+//! assert!(result.is_err());
+//! # Ok::<(), Error>(())
+//! ```
+
+use core::ops::Range;
+
+use crate::{
+    bindings,
+    clist_create,
+    error::to_result,
+    interop::list::CListHead,
+    new_mutex,
+    prelude::*,
+    ptr::Alignment,
+    sync::{
+        lock::mutex::MutexGuard,
+        Arc,
+        Mutex, //
+    },
+    types::Opaque, //
+};
+
+/// Allocation mode for the GPU buddy allocator.
+///
+/// The mode determines the primary allocation strategy. Modes are mutually
+/// exclusive: an allocation is either simple, range-constrained, or top-down.
+///
+/// Orthogonal modifier flags (e.g., contiguous, clear) are specified separately
+/// via [`GpuBuddyAllocFlags`].
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum GpuBuddyAllocMode {
+    /// Simple allocation without constraints.
+    Simple,
+    /// Range-based allocation within the given address range.
+    Range(Range<u64>),
+    /// Allocate from top of address space downward.
+    TopDown,
+}
+
+impl GpuBuddyAllocMode {
+    // Returns the C flags corresponding to the allocation mode.
+    fn as_flags(&self) -> usize {
+        match self {
+            Self::Simple => 0,
+            Self::Range(_) => bindings::GPU_BUDDY_RANGE_ALLOCATION,
+            Self::TopDown => bindings::GPU_BUDDY_TOPDOWN_ALLOCATION,
+        }
+    }
+
+    // Extracts the range start/end, defaulting to `(0, 0)` for non-range modes.
+    fn range(&self) -> (u64, u64) {
+        match self {
+            Self::Range(range) => (range.start, range.end),
+            _ => (0, 0),
+        }
+    }
+}
+
+crate::impl_flags!(
+    /// Modifier flags for GPU buddy allocation.
+    ///
+    /// These flags can be combined with any [`GpuBuddyAllocMode`] to control
+    /// additional allocation behavior.
+    #[derive(Clone, Copy, Default, PartialEq, Eq)]
+    pub struct GpuBuddyAllocFlags(usize);
+
+    /// Individual modifier flag for GPU buddy allocation.
+    #[derive(Clone, Copy, PartialEq, Eq)]
+    pub enum GpuBuddyAllocFlag {
+        /// Allocate physically contiguous blocks.
+        Contiguous = bindings::GPU_BUDDY_CONTIGUOUS_ALLOCATION,
+
+        /// Request allocation from cleared (zeroed) memory.
+        Clear = bindings::GPU_BUDDY_CLEAR_ALLOCATION,
+
+        /// Disable trimming of partially used blocks.
+        TrimDisable = bindings::GPU_BUDDY_TRIM_DISABLE,
+    }
+);
+
+/// Parameters for creating a GPU buddy allocator.
+pub struct GpuBuddyParams {
+    /// Base offset (in bytes) where the managed memory region starts.
+    /// Allocations will be offset by this value.
+    pub base_offset: u64,
+    /// Total size (in bytes) of the address space managed by the allocator.
+    pub size: u64,
+    /// Minimum allocation unit / chunk size, must be >= 4KB.
+    pub chunk_size: Alignment,
+}
+
+/// Inner structure holding the actual buddy allocator.
+///
+/// # Synchronization
+///
+/// The C `gpu_buddy` API requires synchronization (see `include/linux/gpu_buddy.h`).
+/// Internal locking ensures all allocator and free operations are properly
+/// synchronized, preventing races between concurrent allocations and the
+/// freeing that occurs when [`AllocatedBlocks`] is dropped.
+///
+/// # Invariants
+///
+/// The inner [`Opaque`] contains an initialized buddy allocator.
+#[pin_data(PinnedDrop)]
+struct GpuBuddyInner {
+    #[pin]
+    inner: Opaque<bindings::gpu_buddy>,
+
+    // TODO: Replace `Mutex<()>` with `Mutex<Opaque<..>>` once `Mutex::new()`
+    // accepts `impl PinInit<T>`.
+    #[pin]
+    lock: Mutex<()>,
+    /// Cached creation parameters (do not change after init).
+    params: GpuBuddyParams,
+}
+
+impl GpuBuddyInner {
+    /// Create a pin-initializer for the buddy allocator.
+    fn new(params: GpuBuddyParams) -> impl PinInit<Self, Error> {
+        let size = params.size;
+        let chunk_size = params.chunk_size;
+
+        // INVARIANT: `gpu_buddy_init` returns 0 on success, at which point the
+        // `gpu_buddy` structure is initialized and ready for use with all
+        // `gpu_buddy_*` APIs. `try_pin_init!` only completes if all fields succeed,
+        // so the invariant holds when construction finishes.
+        try_pin_init!(Self {
+            inner <- Opaque::try_ffi_init(|ptr| {
+                // SAFETY: `ptr` points to valid uninitialized memory from the pin-init
+                // infrastructure. `gpu_buddy_init` will initialize the structure.
+                to_result(unsafe { bindings::gpu_buddy_init(ptr, size, chunk_size.as_usize() as u64) })
+            }),
+            lock <- new_mutex!(()),
+            params,
+        })
+    }
+
+    /// Lock the mutex and return a guard for accessing the allocator.
+    fn lock(&self) -> GpuBuddyGuard<'_> {
+        GpuBuddyGuard {
+            inner: self,
+            _guard: self.lock.lock(),
+        }
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for GpuBuddyInner {
+    fn drop(self: Pin<&mut Self>) {
+        let guard = self.lock();
+
+        // SAFETY: Per the type invariant, `inner` contains an initialized
+        // allocator. `guard` provides exclusive access.
+        unsafe {
+            bindings::gpu_buddy_fini(guard.as_raw());
+        }
+    }
+}
+
+// SAFETY: `GpuBuddyInner` can be sent between threads.
+unsafe impl Send for GpuBuddyInner {}
+
+// SAFETY: `GpuBuddyInner` is `Sync` because `GpuBuddyInner::lock`
+// serializes all access to the C allocator, preventing data races.
+unsafe impl Sync for GpuBuddyInner {}
+
+// Guard that proves the lock is held, enabling access to the allocator.
+// The `_guard` holds the lock for the duration of this guard's lifetime.
+struct GpuBuddyGuard<'a> {
+    inner: &'a GpuBuddyInner,
+    _guard: MutexGuard<'a, ()>,
+}
+
+impl GpuBuddyGuard<'_> {
+    /// Get a raw pointer to the underlying C `gpu_buddy` structure.
+    fn as_raw(&self) -> *mut bindings::gpu_buddy {
+        self.inner.inner.get()
+    }
+}
+
+/// GPU buddy allocator instance.
+///
+/// This structure wraps the C `gpu_buddy` allocator using reference counting.
+/// The allocator is automatically cleaned up when all references are dropped.
+///
+/// Refer to the module-level documentation for usage examples.
+pub struct GpuBuddy(Arc<GpuBuddyInner>);
+
+impl GpuBuddy {
+    /// Create a new buddy allocator.
+    ///
+    /// Creates a buddy allocator that manages a contiguous address space of the given
+    /// size, with the specified minimum allocation unit (chunk_size must be at least 4KB).
+    pub fn new(params: GpuBuddyParams) -> Result<Self> {
+        Arc::pin_init(GpuBuddyInner::new(params), GFP_KERNEL).map(Self)
+    }
+
+    /// Get the base offset for allocations.
+    pub fn base_offset(&self) -> u64 {
+        self.0.params.base_offset
+    }
+
+    /// Get the chunk size (minimum allocation unit).
+    pub fn chunk_size(&self) -> Alignment {
+        self.0.params.chunk_size
+    }
+
+    /// Get the total managed size.
+    pub fn size(&self) -> u64 {
+        self.0.params.size
+    }
+
+    /// Get the available (free) memory in bytes.
+    pub fn avail(&self) -> u64 {
+        let guard = self.0.lock();
+
+        // SAFETY: Per the type invariant, `inner` contains an initialized allocator.
+        // `guard` provides exclusive access.
+        unsafe { (*guard.as_raw()).avail }
+    }
+
+    /// Allocate blocks from the buddy allocator.
+    ///
+    /// Returns a pin-initializer for [`AllocatedBlocks`].
+    pub fn alloc_blocks(
+        &self,
+        mode: GpuBuddyAllocMode,
+        size: u64,
+        min_block_size: Alignment,
+        flags: impl Into<GpuBuddyAllocFlags>,
+    ) -> impl PinInit<AllocatedBlocks, Error> {
+        let buddy_arc = Arc::clone(&self.0);
+        let (start, end) = mode.range();
+        let mode_flags = mode.as_flags();
+        let modifier_flags = flags.into();
+
+        // Create pin-initializer that initializes list and allocates blocks.
+        try_pin_init!(AllocatedBlocks {
+            buddy: buddy_arc,
+            list <- CListHead::new(),
+            _: {
+                // Reject zero-sized or inverted ranges.
+                if let GpuBuddyAllocMode::Range(range) = &mode {
+                    if range.is_empty() {
+                        Err::<(), Error>(EINVAL)?;
+                    }
+                }
+
+                // Lock while allocating to serialize with concurrent frees.
+                let guard = buddy.lock();
+
+                // SAFETY: Per the type invariant, `inner` contains an initialized
+                // allocator. `guard` provides exclusive access.
+                to_result(unsafe {
+                    bindings::gpu_buddy_alloc_blocks(
+                        guard.as_raw(),
+                        start,
+                        end,
+                        size,
+                        min_block_size.as_usize() as u64,
+                        list.as_raw(),
+                        mode_flags | usize::from(modifier_flags),
+                    )
+                })?
+            }
+        })
+    }
+}
+
+/// Allocated blocks from the buddy allocator with automatic cleanup.
+///
+/// This structure owns a list of allocated blocks and ensures they are
+/// automatically freed when dropped. Use `iter()` to iterate over all
+/// allocated blocks.
+///
+/// # Invariants
+///
+/// - `list` is an initialized, valid list head containing allocated blocks.
+#[pin_data(PinnedDrop)]
+pub struct AllocatedBlocks {
+    #[pin]
+    list: CListHead,
+    buddy: Arc<GpuBuddyInner>,
+}
+
+impl AllocatedBlocks {
+    /// Check if the block list is empty.
+    pub fn is_empty(&self) -> bool {
+        // An empty list head points to itself.
+        !self.list.is_linked()
+    }
+
+    /// Iterate over allocated blocks.
+    ///
+    /// Returns an iterator yielding [`AllocatedBlock`] values. Each [`AllocatedBlock`]
+    /// borrows `self` and is only valid for the duration of that borrow.
+    pub fn iter(&self) -> impl Iterator<Item = AllocatedBlock<'_>> + '_ {
+        let head = self.list.as_raw();
+        // SAFETY: Per the type invariant, `list` is an initialized sentinel `list_head`
+        // and is not concurrently modified (we hold a `&self` borrow). The list contains
+        // `gpu_buddy_block` items linked via `__bindgen_anon_1.link`. `Block` is
+        // `#[repr(transparent)]` over `gpu_buddy_block`.
+        let clist = unsafe {
+            clist_create!(
+                head,
+                Block,
+                bindings::gpu_buddy_block,
+                __bindgen_anon_1.link
+            )
+        };
+
+        clist
+            .iter()
+            .map(|this| AllocatedBlock { this, blocks: self })
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for AllocatedBlocks {
+    fn drop(self: Pin<&mut Self>) {
+        let guard = self.buddy.lock();
+
+        // SAFETY:
+        // - list is valid per the type's invariants.
+        // - guard provides exclusive access to the allocator.
+        unsafe {
+            bindings::gpu_buddy_free_list(guard.as_raw(), self.list.as_raw(), 0);
+        }
+    }
+}
+
+/// A GPU buddy block.
+///
+/// Transparent wrapper over C `gpu_buddy_block` structure. This type is returned
+/// as references during iteration over [`AllocatedBlocks`].
+///
+/// # Invariants
+///
+/// The inner [`Opaque`] contains a valid, allocated `gpu_buddy_block`.
+#[repr(transparent)]
+struct Block(Opaque<bindings::gpu_buddy_block>);
+
+impl Block {
+    /// Get a raw pointer to the underlying C block.
+    fn as_raw(&self) -> *mut bindings::gpu_buddy_block {
+        self.0.get()
+    }
+
+    /// Get the block's raw offset in the buddy address space (without base offset).
+    fn offset(&self) -> u64 {
+        // SAFETY: `self.as_raw()` is valid per the type's invariants.
+        unsafe { bindings::gpu_buddy_block_offset(self.as_raw()) }
+    }
+
+    /// Get the block order.
+    fn order(&self) -> u32 {
+        // SAFETY: `self.as_raw()` is valid per the type's invariants.
+        unsafe { bindings::gpu_buddy_block_order(self.as_raw()) }
+    }
+}
+
+// SAFETY: `Block` is a wrapper around `gpu_buddy_block` which can be
+// sent across threads safely.
+unsafe impl Send for Block {}
+
+// SAFETY: `Block` is only accessed through shared references after
+// allocation, and thus safe to access concurrently across threads.
+unsafe impl Sync for Block {}
+
+/// A buddy block paired with its owning [`AllocatedBlocks`] context.
+///
+/// Unlike a raw block, which only knows its offset within the buddy address
+/// space, an [`AllocatedBlock`] also has access to the allocator's `base_offset`
+/// and `chunk_size`, enabling it to compute absolute offsets and byte sizes.
+///
+/// Returned by [`AllocatedBlocks::iter()`].
+pub struct AllocatedBlock<'a> {
+    this: &'a Block,
+    blocks: &'a AllocatedBlocks,
+}
+
+impl AllocatedBlock<'_> {
+    /// Get the block's offset in the address space.
+    ///
+    /// Returns the absolute offset including the allocator's base offset.
+    /// This is the actual address to use for accessing the allocated memory.
+    pub fn offset(&self) -> u64 {
+        self.blocks.buddy.params.base_offset + self.this.offset()
+    }
+
+    /// Get the block order (size = chunk_size << order).
+    pub fn order(&self) -> u32 {
+        self.this.order()
+    }
+
+    /// Get the block's size in bytes.
+    pub fn size(&self) -> u64 {
+        (self.blocks.buddy.params.chunk_size.as_usize() as u64) << self.this.order()
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index bdcf632050ee..2652933e585f 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -102,6 +102,8 @@
 pub mod firmware;
 pub mod fmt;
 pub mod fs;
+#[cfg(CONFIG_GPU_BUDDY = "y")]
+pub mod gpu;
 #[cfg(CONFIG_I2C = "y")]
 pub mod i2c;
 pub mod id_pool;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v14 2/2] MAINTAINERS: gpu: buddy: Update reviewer
From: Alexandre Courbot @ 2026-03-20 12:16 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
	Koen Koning, dri-devel, rust-for-linux, Nikola Djukic,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jonathan Corbet, Alex Deucher,
	Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller, Alex Gaynor,
	Boqun Feng, John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	Philipp Stanner, Elle Rhumsaa, alexeyi, Eliot Courtney, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260320045711.43494-3-joelagnelf@nvidia.com>

On Fri Mar 20, 2026 at 1:57 PM JST, Joel Fernandes wrote:
> Christian Koenig mentioned he'd like to step down from the reviewer
> role for the GPU buddy allocator. Joel Fernandes is stepping in as
> reviewer with agreement from Matthew Auld and Arun Pravin.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

This is missing the Acked-bys you collected on v13.

^ permalink raw reply

* Re: [PATCH v14 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Alexandre Courbot @ 2026-03-20 12:16 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
	Koen Koning, dri-devel, rust-for-linux, Nikola Djukic,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jonathan Corbet, Alex Deucher,
	Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller, Alex Gaynor,
	Boqun Feng, John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	Philipp Stanner, Elle Rhumsaa, alexeyi, Eliot Courtney, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260320045711.43494-2-joelagnelf@nvidia.com>

On Fri Mar 20, 2026 at 1:57 PM JST, Joel Fernandes wrote:
> Add safe Rust abstractions over the Linux kernel's GPU buddy
> allocator for physical memory management. The GPU buddy allocator
> implements a binary buddy system useful for GPU physical memory
> allocation. nova-core will use it for physical memory allocation.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

^ permalink raw reply

* Re: [PATCH v14 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Gary Guo @ 2026-03-20 13:04 UTC (permalink / raw)
  To: Joel Fernandes, linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
	dri-devel, rust-for-linux, Nikola Djukic, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Alex Gaynor, Boqun Feng,
	John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi,
	Eliot Courtney, joel, linux-doc, amd-gfx, intel-gfx, intel-xe,
	linux-fbdev
In-Reply-To: <20260320045711.43494-2-joelagnelf@nvidia.com>

On Fri Mar 20, 2026 at 4:57 AM GMT, Joel Fernandes wrote:
> Add safe Rust abstractions over the Linux kernel's GPU buddy
> allocator for physical memory management. The GPU buddy allocator
> implements a binary buddy system useful for GPU physical memory
> allocation. nova-core will use it for physical memory allocation.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  MAINTAINERS                     |   6 +
>  rust/bindings/bindings_helper.h |  11 +
>  rust/helpers/gpu.c              |  23 ++
>  rust/helpers/helpers.c          |   1 +
>  rust/kernel/gpu.rs              |   6 +
>  rust/kernel/gpu/buddy.rs        | 613 ++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   2 +
>  7 files changed, 662 insertions(+)
>  create mode 100644 rust/helpers/gpu.c
>  create mode 100644 rust/kernel/gpu.rs
>  create mode 100644 rust/kernel/gpu/buddy.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e847099efcc2..cd9505d3be60 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8531,7 +8531,10 @@ T:	git https://gitlab.freedesktop.org/drm/rust/kernel.git
>  F:	drivers/gpu/drm/nova/
>  F:	drivers/gpu/drm/tyr/
>  F:	drivers/gpu/nova-core/
> +F:	rust/helpers/gpu.c
>  F:	rust/kernel/drm/
> +F:	rust/kernel/gpu.rs
> +F:	rust/kernel/gpu/
>  
>  DRM DRIVERS FOR ALLWINNER A10
>  M:	Chen-Yu Tsai <wens@kernel.org>
> @@ -8952,6 +8955,9 @@ F:	drivers/gpu/drm/drm_buddy.c
>  F:	drivers/gpu/tests/gpu_buddy_test.c
>  F:	include/drm/drm_buddy.h
>  F:	include/linux/gpu_buddy.h
> +F:	rust/helpers/gpu.c
> +F:	rust/kernel/gpu.rs
> +F:	rust/kernel/gpu/
>  
>  DRM AUTOMATED TESTING
>  M:	Helen Koike <helen.fornazier@gmail.com>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 083cc44aa952..dbb765a9fdbd 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -29,6 +29,7 @@
>  #include <linux/hrtimer_types.h>
>  
>  #include <linux/acpi.h>
> +#include <linux/gpu_buddy.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_file.h>
> @@ -146,6 +147,16 @@ const vm_flags_t RUST_CONST_HELPER_VM_MIXEDMAP = VM_MIXEDMAP;
>  const vm_flags_t RUST_CONST_HELPER_VM_HUGEPAGE = VM_HUGEPAGE;
>  const vm_flags_t RUST_CONST_HELPER_VM_NOHUGEPAGE = VM_NOHUGEPAGE;
>  
> +#if IS_ENABLED(CONFIG_GPU_BUDDY)
> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_RANGE_ALLOCATION = GPU_BUDDY_RANGE_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TOPDOWN_ALLOCATION = GPU_BUDDY_TOPDOWN_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CONTIGUOUS_ALLOCATION =
> +								GPU_BUDDY_CONTIGUOUS_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEAR_ALLOCATION = GPU_BUDDY_CLEAR_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEARED = GPU_BUDDY_CLEARED;
> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TRIM_DISABLE = GPU_BUDDY_TRIM_DISABLE;
> +#endif
> +
>  #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
>  #include "../../drivers/android/binder/rust_binder.h"
>  #include "../../drivers/android/binder/rust_binder_events.h"
> diff --git a/rust/helpers/gpu.c b/rust/helpers/gpu.c
> new file mode 100644
> index 000000000000..38b1a4e6bef8
> --- /dev/null
> +++ b/rust/helpers/gpu.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/gpu_buddy.h>
> +
> +#ifdef CONFIG_GPU_BUDDY
> +
> +__rust_helper u64 rust_helper_gpu_buddy_block_offset(const struct gpu_buddy_block *block)
> +{
> +	return gpu_buddy_block_offset(block);
> +}
> +
> +__rust_helper unsigned int rust_helper_gpu_buddy_block_order(struct gpu_buddy_block *block)
> +{
> +	return gpu_buddy_block_order(block);
> +}
> +
> +__rust_helper u64 rust_helper_gpu_buddy_block_size(struct gpu_buddy *mm,
> +						   struct gpu_buddy_block *block)
> +{
> +	return gpu_buddy_block_size(mm, block);
> +}

From Sashiko:
https://sashiko.dev/#/patchset/20260320045711.43494-1-joelagnelf%40nvidia.com

    Does the Rust wrapper use this helper? It looks like AllocatedBlock::size()
    manually duplicates the bitwise logic (chunk_size << order) rather than
    calling this helper, which could create a divergence risk if the underlying C
    allocator implementation changes.

Many other review comments there seem to be false positive, but it might worth
confirming.

Best,
Gary

> +
> +#endif /* CONFIG_GPU_BUDDY */
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 724fcb8240ac..a53929ce52a3 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -32,6 +32,7 @@
>  #include "err.c"
>  #include "irq.c"
>  #include "fs.c"
> +#include "gpu.c"
>  #include "io.c"
>  #include "jump_label.c"
>  #include "kunit.c"
> diff --git a/rust/kernel/gpu.rs b/rust/kernel/gpu.rs
> new file mode 100644
> index 000000000000..1dc5d0c8c09d
> --- /dev/null
> +++ b/rust/kernel/gpu.rs
> @@ -0,0 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! GPU subsystem abstractions.
> +
> +#[cfg(CONFIG_GPU_BUDDY = "y")]
> +pub mod buddy;


^ permalink raw reply

* [PATCH v1 1/1] video: fbdev: matroxfb: Mark variable with __maybe_unused to avoid W=1 build break
From: Andy Shevchenko @ 2026-03-20 14:36 UTC (permalink / raw)
  To: Thomas Zimmermann, linux-fbdev, dri-devel, linux-kernel, llvm
  Cc: Helge Deller, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Andy Shevchenko, Jason Yan

Clang is not happy about set but unused variable:

drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
   412 |                                 unsigned int mnp;
       |                                              ^
1 error generated.

Since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
and set but not used variable") the 'mnp' became unused, but eliminating
that code might have side-effects. The question here is what should we do
with 'mnp'? The easiest way out is just mark it with __maybe_unused which
will shut the compiler up and won't change any possible IO flow. So does
this change.

Fixes: 7b987887f97b ("video: fbdev: matroxfb: remove dead code and set but not used variable")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

---
Cc: Jason Yan <yanaijie@huawei.com>
---

Below is a dive into the history of the driver.

The problem was revealed when the #if 0 guarded code along with unused
pixel_vco variable was removed. That code was introduced in the original
commit 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded
in the commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on
G450/G550. Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC
from older DAC1064 handling. Modify PLL setting when both CRTCs use same
pixel clocks.").

NOTE: The two commits mentioned above pre-date Git era and available in
history.git repository for archaeological purposes.

Even without that guard the modern compilers may see that the pixel_vco
wasn't ever used and seems a leftover after some debug or review made
25 years ago.

The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem
to have any side effects either than some unneeded CPU processing during
runtime. I agree that's unlikely that timeout (or heating up the CPU) has
any effect on the HW (GPU/display) functionality.
---
 drivers/video/fbdev/matrox/g450_pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
index e2c1478aa47f..6a08f78cd1ac 100644
--- a/drivers/video/fbdev/matrox/g450_pll.c
+++ b/drivers/video/fbdev/matrox/g450_pll.c
@@ -409,7 +409,7 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
 		case M_VIDEO_PLL:
 			{
 				u_int8_t tmp;
-				unsigned int mnp;
+				unsigned int mnp __maybe_unused;
 				unsigned long flags;
 
 				matroxfb_DAC_lock_irqsave(flags);
-- 
2.50.1


^ permalink raw reply related

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Andy Shevchenko @ 2026-03-20 14:37 UTC (permalink / raw)
  To: Helge Deller
  Cc: Jason Yan, Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <f13eab5d-bcf0-49ba-91ac-0903040438be@gmx.de>

On Thu, Mar 19, 2026 at 01:44:28PM +0100, Helge Deller wrote:
> On 3/19/26 10:08, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
> > > On 3/19/26 09:21, Andy Shevchenko wrote:
> > > > On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> > > > > 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > > > > > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:

...

> > > > > > Helge, can we revert this change and start over, please? (I can also send
> > > > > > revert if you think it's a better way)
> > > 
> > > Andy, all points you make against removing relevant code is absolutely right.
> > > 
> > > But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
> > > set but not used variable") I have to agree with Jason, that the patch is ok.
> > > I don't see any build errors either.
> > 
> > Just run on today's Linux Next (since the driver has not been changed in that
> > sense for a few years, the very same issue is present for a long time):
> > 
> > drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
> >    412 |                                 unsigned int mnp;
> >        |                                              ^
> > 1 error generated.
> 
> Ok.
> 
> > > Are we mixing up things here maybe?
> > 
> > ...
> > 
> > FWIW, I dove to the history of the driver.
> > 
> > So, the code, that was guarded by #if 0 was introduced in the original commit
> > 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded in the
> > commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on G450/G550.
> > Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC from older
> > DAC1064 handling. Modify PLL setting when both CRTCs use same pixel clocks.").
> > 
> > Even without that guard the modern compilers may see that the pixel_vco wasn't
> > ever used and seems a leftover after some debug or review made 25 years ago.
> > 
> > The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem to have
> > any side effects either than some unneeded CPU processing during runtime. I
> > agree that's unlikely that timeout (or heating up the CPU) has any effect on
> > the HW (GPU/display) functionality.
> > 
> > So, since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
> > and set but not used variable") the 'mnp' became unused, but eliminating that
> > code might have side-effects. The question here is what should we do with mnp?
> > The easiest way out is just mark it with __maybe_unused which will shut the
> > compiler up and won't change any possible IO flow.
> 
> Yes, I think that's what I'd prefer.
> Do you mind sending a patch?

I have just sent 20260320143646.3218199-1-andriy.shevchenko@linux.intel.com.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v1 1/1] video: fbdev: matroxfb: Mark variable with __maybe_unused to avoid W=1 build break
From: Helge Deller @ 2026-03-20 16:32 UTC (permalink / raw)
  To: Andy Shevchenko, Thomas Zimmermann, linux-fbdev, dri-devel,
	linux-kernel, llvm
  Cc: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Jason Yan
In-Reply-To: <20260320143646.3218199-1-andriy.shevchenko@linux.intel.com>

On 3/20/26 15:36, Andy Shevchenko wrote:
> Clang is not happy about set but unused variable:
> 
> drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
>     412 |                                 unsigned int mnp;
>         |                                              ^
> 1 error generated.
> 
> Since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
> and set but not used variable") the 'mnp' became unused, but eliminating
> that code might have side-effects. The question here is what should we do
> with 'mnp'? The easiest way out is just mark it with __maybe_unused which
> will shut the compiler up and won't change any possible IO flow. So does
> this change.
> 
> Fixes: 7b987887f97b ("video: fbdev: matroxfb: remove dead code and set but not used variable")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Helge Deller <deller@gmx.de>

Thanks!
Helge

> 
> ---
> Cc: Jason Yan <yanaijie@huawei.com>
> ---
> 
> Below is a dive into the history of the driver.
> 
> The problem was revealed when the #if 0 guarded code along with unused
> pixel_vco variable was removed. That code was introduced in the original
> commit 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded
> in the commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on
> G450/G550. Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC
> from older DAC1064 handling. Modify PLL setting when both CRTCs use same
> pixel clocks.").
> 
> NOTE: The two commits mentioned above pre-date Git era and available in
> history.git repository for archaeological purposes.
> 
> Even without that guard the modern compilers may see that the pixel_vco
> wasn't ever used and seems a leftover after some debug or review made
> 25 years ago.
> 
> The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem
> to have any side effects either than some unneeded CPU processing during
> runtime. I agree that's unlikely that timeout (or heating up the CPU) has
> any effect on the HW (GPU/display) functionality.
> ---
>   drivers/video/fbdev/matrox/g450_pll.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
> index e2c1478aa47f..6a08f78cd1ac 100644
> --- a/drivers/video/fbdev/matrox/g450_pll.c
> +++ b/drivers/video/fbdev/matrox/g450_pll.c
> @@ -409,7 +409,7 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
>   		case M_VIDEO_PLL:
>   			{
>   				u_int8_t tmp;
> -				unsigned int mnp;
> +				unsigned int mnp __maybe_unused;
>   				unsigned long flags;
>   
>   				matroxfb_DAC_lock_irqsave(flags);


^ permalink raw reply

* Re: fbdev: update outdated help text for CONFIG_FB_NVIDIA
From: Helge Deller @ 2026-03-20 16:56 UTC (permalink / raw)
  To: robgithub, Randy Dunlap; +Cc: linux-fbdev, dri-devel
In-Reply-To: <20260313203443.7cf0a8f5@hexa5>

On 3/13/26 21:35, robgithub wrote:
>  From 688a061ba0db71fc2d5facd8344db7a4d5b1575a Mon Sep 17 00:00:00 2001
> From: robgithub <rob.github@jumpstation.co.uk>
> Date: Wed, 11 Mar 2026 22:14:43 +0000
> Subject: [PATCH] fbdev: update outdated help text for CONFIG_FB_NVIDIA
> 
> The help text for CONFIG_FB_NVIDIA refers to obsolete hardware and
> incorrect default behaviour. This patch updates the description to
> reflect the current state of the driver and supported devices.
> 
> Signed-off-by: robgithub <rob.github@jumpstation.co.uk>

applied with minor modifications.

Thanks!
Helge

^ permalink raw reply

* Re: [PATCH v14 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Joel Fernandes @ 2026-03-20 19:50 UTC (permalink / raw)
  To: Gary Guo, linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, Alex Gaynor, Boqun Feng, John Hubbard,
	Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
	Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	Philipp Stanner, Elle Rhumsaa, alexeyi, Eliot Courtney, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH7MPTIK5OMK.3GHQAE07J5OO@garyguo.net>



On 3/20/2026 9:04 AM, Gary Guo wrote:
> On Fri Mar 20, 2026 at 4:57 AM GMT, Joel Fernandes wrote:
>> Add safe Rust abstractions over the Linux kernel's GPU buddy
>> allocator for physical memory management. The GPU buddy allocator
>> implements a binary buddy system useful for GPU physical memory
>> allocation. nova-core will use it for physical memory allocation.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  MAINTAINERS                     |   6 +
>>  rust/bindings/bindings_helper.h |  11 +
>>  rust/helpers/gpu.c              |  23 ++
>>  rust/helpers/helpers.c          |   1 +
>>  rust/kernel/gpu.rs              |   6 +
>>  rust/kernel/gpu/buddy.rs        | 613 ++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs              |   2 +
>>  7 files changed, 662 insertions(+)
>>  create mode 100644 rust/helpers/gpu.c
>>  create mode 100644 rust/kernel/gpu.rs
>>  create mode 100644 rust/kernel/gpu/buddy.rs
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e847099efcc2..cd9505d3be60 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8531,7 +8531,10 @@ T:	git https://gitlab.freedesktop.org/drm/rust/kernel.git
>>  F:	drivers/gpu/drm/nova/
>>  F:	drivers/gpu/drm/tyr/
>>  F:	drivers/gpu/nova-core/
>> +F:	rust/helpers/gpu.c
>>  F:	rust/kernel/drm/
>> +F:	rust/kernel/gpu.rs
>> +F:	rust/kernel/gpu/
>>  
>>  DRM DRIVERS FOR ALLWINNER A10
>>  M:	Chen-Yu Tsai <wens@kernel.org>
>> @@ -8952,6 +8955,9 @@ F:	drivers/gpu/drm/drm_buddy.c
>>  F:	drivers/gpu/tests/gpu_buddy_test.c
>>  F:	include/drm/drm_buddy.h
>>  F:	include/linux/gpu_buddy.h
>> +F:	rust/helpers/gpu.c
>> +F:	rust/kernel/gpu.rs
>> +F:	rust/kernel/gpu/
>>  
>>  DRM AUTOMATED TESTING
>>  M:	Helen Koike <helen.fornazier@gmail.com>
>> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>> index 083cc44aa952..dbb765a9fdbd 100644
>> --- a/rust/bindings/bindings_helper.h
>> +++ b/rust/bindings/bindings_helper.h
>> @@ -29,6 +29,7 @@
>>  #include <linux/hrtimer_types.h>
>>  
>>  #include <linux/acpi.h>
>> +#include <linux/gpu_buddy.h>
>>  #include <drm/drm_device.h>
>>  #include <drm/drm_drv.h>
>>  #include <drm/drm_file.h>
>> @@ -146,6 +147,16 @@ const vm_flags_t RUST_CONST_HELPER_VM_MIXEDMAP = VM_MIXEDMAP;
>>  const vm_flags_t RUST_CONST_HELPER_VM_HUGEPAGE = VM_HUGEPAGE;
>>  const vm_flags_t RUST_CONST_HELPER_VM_NOHUGEPAGE = VM_NOHUGEPAGE;
>>  
>> +#if IS_ENABLED(CONFIG_GPU_BUDDY)
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_RANGE_ALLOCATION = GPU_BUDDY_RANGE_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TOPDOWN_ALLOCATION = GPU_BUDDY_TOPDOWN_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CONTIGUOUS_ALLOCATION =
>> +								GPU_BUDDY_CONTIGUOUS_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEAR_ALLOCATION = GPU_BUDDY_CLEAR_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEARED = GPU_BUDDY_CLEARED;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TRIM_DISABLE = GPU_BUDDY_TRIM_DISABLE;
>> +#endif
>> +
>>  #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
>>  #include "../../drivers/android/binder/rust_binder.h"
>>  #include "../../drivers/android/binder/rust_binder_events.h"
>> diff --git a/rust/helpers/gpu.c b/rust/helpers/gpu.c
>> new file mode 100644
>> index 000000000000..38b1a4e6bef8
>> --- /dev/null
>> +++ b/rust/helpers/gpu.c
>> @@ -0,0 +1,23 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/gpu_buddy.h>
>> +
>> +#ifdef CONFIG_GPU_BUDDY
>> +
>> +__rust_helper u64 rust_helper_gpu_buddy_block_offset(const struct gpu_buddy_block *block)
>> +{
>> +	return gpu_buddy_block_offset(block);
>> +}
>> +
>> +__rust_helper unsigned int rust_helper_gpu_buddy_block_order(struct gpu_buddy_block *block)
>> +{
>> +	return gpu_buddy_block_order(block);
>> +}
>> +
>> +__rust_helper u64 rust_helper_gpu_buddy_block_size(struct gpu_buddy *mm,
>> +						   struct gpu_buddy_block *block)
>> +{
>> +	return gpu_buddy_block_size(mm, block);
>> +}
> 
> From Sashiko:
> https://sashiko.dev/#/patchset/20260320045711.43494-1-joelagnelf%40nvidia.com
> 
>     Does the Rust wrapper use this helper? It looks like AllocatedBlock::size()
>     manually duplicates the bitwise logic (chunk_size << order) rather than
>     calling this helper, which could create a divergence risk if the underlying C
>     allocator implementation changes.
> 
> Many other review comments there seem to be false positive, but it might worth
> confirming.
If Danilo is applying the patch, can we please remove the helper on apply? I
think in this case we do not need to call the helper. I don't think there is a
divergence risk here.

thanks,

--
Joel Fernandes


^ permalink raw reply

* Re: [PATCH v14 2/2] MAINTAINERS: gpu: buddy: Update reviewer
From: Joel Fernandes @ 2026-03-21  0:06 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: linux-kernel@vger.kernel.org, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
	Koen Koning, dri-devel@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org, Nikola Djukic, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Vivi Rodrigo, Tvrtko Ursulin, Rui Huang,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Alex Gaynor, Boqun Feng,
	John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	Philipp Stanner, Elle Rhumsaa, Alexey Ivanov, Eliot Courtney,
	joel@joelfernandes.org, linux-doc@vger.kernel.org,
	amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org, linux-fbdev@vger.kernel.org
In-Reply-To: <DH7LP20OY5TJ.3ICBGXWHA7LQV@nvidia.com>



> On Mar 20, 2026, at 8:16 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 
> On Fri Mar 20, 2026 at 1:57 PM JST, Joel Fernandes wrote:
>> Christian Koenig mentioned he'd like to step down from the reviewer
>> role for the GPU buddy allocator. Joel Fernandes is stepping in as
>> reviewer with agreement from Matthew Auld and Arun Pravin.
>> 
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> 
> This is missing the Acked-bys you collected on v13.

Thank you, would Danilo be willing to add it only apply so I do not need to resend? Acks are from Mathew, Arun and Christian.




^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox