* [PATCH v8 2/2] staging: fbtft: Make framebuffer registration message debug-only
From: Chintan Patel @ 2026-01-22 3:16 UTC (permalink / raw)
To: linux-fbdev, linux-staging, linux-omap
Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
Chintan Patel, Andy Shevchenko
In-Reply-To: <20260122031635.11414-1-chintanlike@gmail.com>
The framebuffer registration message is informational only and not
useful during normal operation. Convert it to debug-level logging to
keep the driver quiet when working correctly.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
Changes in v8:
- Add Reviewed-by tag from Andy Shevchenko
- Add Suggested-by tag from Greg Kroah-Hartman
drivers/staging/fbtft/fbtft-core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 1b3b62950205..f427c0914907 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -792,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
if (spi)
sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
- fb_info(fb_info,
- "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
- fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
- fb_info->fix.smem_len >> 10, text1,
- HZ / fb_info->fbdefio->delay, text2);
+ fb_dbg(fb_info,
+ "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
+ fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
+ fb_info->fix.smem_len >> 10, text1,
+ HZ / fb_info->fbdefio->delay, text2);
/* Turn on backlight if available */
if (fb_info->bl_dev) {
--
2.43.0
^ permalink raw reply related
* [PATCH v8 1/2] staging: fbtft: Fix build failure when CONFIG_FB_DEVICE=n
From: Chintan Patel @ 2026-01-22 3:16 UTC (permalink / raw)
To: linux-fbdev, linux-staging, linux-omap
Cc: linux-kernel, dri-devel, tzimmermann, andy, deller, gregkh,
Chintan Patel, kernel test robot, Andy Shevchenko
When CONFIG_FB_DEVICE is disabled, struct fb_info does
not provide a valid dev pointer. Direct dereferences of
fb_info->dev therefore result in build failures.
Fix this by avoiding direct accesses to fb_info->dev and
switching the affected debug logging to framebuffer helpers
that do not rely on a device pointer.
This fixes the following build failure reported by the
kernel test robot.
Fixes: a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202601110740.Y9XK5HtN-lkp@intel.com
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
Changes in v8:
- Add Reviewed-by tag from Andy Shevchenko.
Changes in v7:
- Split logging cleanups into a separate patch
- Limit this patch to the CONFIG_FB_DEVICE=n build fix only
Changes in v6:
- Switch debug/info logging to fb_dbg() and fb_info()(suggested by Thomas Zimmermann)
- Drop dev_of_fbinfo() usage in favor of framebuffer helpers that implicitly
handle the debug/info context.
- Drop __func__ usage per review feedback(suggested by greg k-h)
- Add Fixes tag for a06d03f9f238 ("staging: fbtft: Make FB_DEVICE dependency optional")
(suggested by Andy Shevchenko)
Changes in v5:
- Initial attempt to replace info->dev accesses using
dev_of_fbinfo() helper
drivers/staging/fbtft/fbtft-core.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..1b3b62950205 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -365,9 +365,9 @@ static int fbtft_fb_setcolreg(unsigned int regno, unsigned int red,
unsigned int val;
int ret = 1;
- dev_dbg(info->dev,
- "%s(regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X)\n",
- __func__, regno, red, green, blue, transp);
+ fb_dbg(info,
+ "regno=%u, red=0x%X, green=0x%X, blue=0x%X, trans=0x%X\n",
+ regno, red, green, blue, transp);
switch (info->fix.visual) {
case FB_VISUAL_TRUECOLOR:
@@ -391,8 +391,7 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)
struct fbtft_par *par = info->par;
int ret = -EINVAL;
- dev_dbg(info->dev, "%s(blank=%d)\n",
- __func__, blank);
+ fb_dbg(info, "blank=%d\n", blank);
if (!par->fbtftops.blank)
return ret;
@@ -793,11 +792,11 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
if (spi)
sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,
spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
- dev_info(fb_info->dev,
- "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
- fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
- fb_info->fix.smem_len >> 10, text1,
- HZ / fb_info->fbdefio->delay, text2);
+ fb_info(fb_info,
+ "%s frame buffer, %dx%d, %d KiB video memory%s, fps=%lu%s\n",
+ fb_info->fix.id, fb_info->var.xres, fb_info->var.yres,
+ fb_info->fix.smem_len >> 10, text1,
+ HZ / fb_info->fbdefio->delay, text2);
/* Turn on backlight if available */
if (fb_info->bl_dev) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-01-21 20:46 UTC (permalink / raw)
To: Gary Guo, linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <DFUK089V1IEU.U83YQT72BO3@garyguo.net>
On 1/21/2026 3:36 PM, Gary Guo wrote:
>> [...]
>>>> +
>>>> +/// Initialize a `list_head` object to point to itself.
>>>> +///
>>>> +/// # Safety
>>>> +///
>>>> +/// `list` must be a valid pointer to a `list_head` object.
>>>> +#[inline]
>>>> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
>>>> + // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
>>>> + unsafe {
>>>> + (*list).next = list;
>>>> + (*list).prev = list;
>>>
>>> This needs to be an atomic write or it'll depart from the C implementation.
>> I am curious what you mean by atomic write, can you define it? Does rust
>> compiler have load/store fusing, invented stores, etc, like C does? Sorry I am
>> only familiar with these concepts on C. Could you provide example of a race
>> condition in Rust that can happen?
>
> Oh yes, this would definitely happen. It's down to LLVM to compile anyway. If
> you create a reference, there'll be even more freedom to do these.
>
Ok.
>> Also I did this addition based on feedback from past review:
>> https://lore.kernel.org/all/DEI89VUEYXAJ.1IQQPC3QRLITP@nvidia.com/
>>
>> There was some concerns around pointless function call overhead when the rust
>> implementation is already quite intertwined with internals of the C linked list
>> implementation. I do agree with that point of view too.
>
> Overall our practice is to not duplicate code. Even `ERR_PTR` is calling into
> helpers.
>
> For performance, it's a valid concern. However Alice and I have series out there
> that enable you to inline the helpers. I'd say unless there's an absolute need,
> we should do the helpers. Especially with caveats like WRITE_ONCE in this case.
Sounds good, so I will then go back to adding a INIT_LIST_HEAD C helper for the
next spin. I agree with the suggestion and now that we are inlining helpers,
there seems little point in adding a separate rust function to do the same.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-01-21 20:41 UTC (permalink / raw)
To: Gary Guo, linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <DFUK089V1IEU.U83YQT72BO3@garyguo.net>
On 1/21/2026 3:36 PM, Gary Guo wrote:
>> There are 2 users:
>>
>> pub fn try_init<E>(
>>
>> and the self-tests:
> This is not really a public user. It's hidden in the doc test too, you could
> initialize using try_init too.
>
>> //! # let head = head.as_mut_ptr();
>> //! # // SAFETY: head and all the items are test objects allocated in [..]
>> //! # unsafe { init_list_head(head) };
>> //! #
True, but if we initialize purely within try_init() without using a helper, does
that not defeat the argument of adding a separate INIT_LIST_HEAD helper such
that we don't deviate from the C side?
Regarding your other comment about the try_init block itself, I will take a look
at your suggestion and see if I can simplify.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Gary Guo @ 2026-01-21 20:36 UTC (permalink / raw)
To: Joel Fernandes, Gary Guo, linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <01a981f1-64c7-4504-b309-45a024258fe9@nvidia.com>
On Wed Jan 21, 2026 at 7:50 PM GMT, Joel Fernandes wrote:
> Hello, Gary,
>
> On 1/20/2026 6:48 PM, Gary Guo wrote:
>> On Tue Jan 20, 2026 at 8:42 PM GMT, Joel Fernandes wrote:
>>> Add a new module `clist` 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.
>>
>> This should read "CList".
>
> Sure, will fix.
>
>>
>> I was quite dubious about the patch just from the title (everybody knows how
>> easy a linked list is in Rust), but it turns out it is not as concerning as I
>> expected, mostly due to the read-only nature of the particular implementation
>> (a lot of the safety comments would be much more difficult to justify, say, if
>> it's mutable). That said, still a lot of feedbacks below.
>
> Sure, the reason for requiring this is interfacing with lists coming from C
> code. I'd see a future where we may want it mutable too (example, Rust code
> adding elements to the existing). At which point, the invariants/safety
> reasoning may change.
>
>> I think something like is okay in the short term. However, there's an growing
>> interest in getting our Rust list API improved, so it could be ideal if
>> eventually the Rust list can be capable of handling FFI lists, too.
>
> Yeah we looked into that, if you see old threads, the conclusion was it is not a
> good fit for existing rust list abstractions. TLDR; it does not fit into their
> ownership/borrowing model.
Definitely not with the existing one that we have, as it handles only `Arc`.
But the existing abstraction is also not good enough if you want to insert
`Box`...
>
> [...]
>>> +
>>> +/// Initialize a `list_head` object to point to itself.
>>> +///
>>> +/// # Safety
>>> +///
>>> +/// `list` must be a valid pointer to a `list_head` object.
>>> +#[inline]
>>> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
>>> + // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
>>> + unsafe {
>>> + (*list).next = list;
>>> + (*list).prev = list;
>>
>> This needs to be an atomic write or it'll depart from the C implementation.
>
> I am curious what you mean by atomic write, can you define it? Does rust
> compiler have load/store fusing, invented stores, etc, like C does? Sorry I am
> only familiar with these concepts on C. Could you provide example of a race
> condition in Rust that can happen?
Oh yes, this would definitely happen. It's down to LLVM to compile anyway. If
you create a reference, there'll be even more freedom to do these.
>
> Also I did this addition based on feedback from past review:
> https://lore.kernel.org/all/DEI89VUEYXAJ.1IQQPC3QRLITP@nvidia.com/
>
> There was some concerns around pointless function call overhead when the rust
> implementation is already quite intertwined with internals of the C linked list
> implementation. I do agree with that point of view too.
Overall our practice is to not duplicate code. Even `ERR_PTR` is calling into
helpers.
For performance, it's a valid concern. However Alice and I have series out there
that enable you to inline the helpers. I'd say unless there's an absolute need,
we should do the helpers. Especially with caveats like WRITE_ONCE in this case.
>
> Also see my other reply to Zhi on this helper topic, lets discuss there too, if
> that's Ok.
>
>>> + }
>>> +}
>>
>> I don't think we want to publicly expose this! I've not found a user in the
>> subsequent patch, too.
>
> There are 2 users:
>
> pub fn try_init<E>(
>
> and the self-tests:
This is not really a public user. It's hidden in the doc test too, you could
initialize using try_init too.
>
> //! # let head = head.as_mut_ptr();
> //! # // SAFETY: head and all the items are test objects allocated in [..]
> //! # unsafe { init_list_head(head) };
> //! #
>
>>
>>> +
>>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>>> +#[repr(transparent)]
>>> +pub struct CListHead(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 allocated and initialized `list_head` structure.
>>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>>> + #[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.
>>> + unsafe { &*ptr.cast() }
>>> + }
>>> +
>>> + /// Get the raw `list_head` pointer.
>>> + #[inline]
>>> + pub fn as_raw(&self) -> *mut bindings::list_head {
>>> + self.0.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 per type invariants.
>>> + // - The `next` pointer is guaranteed to be non-NULL.
>>> + unsafe { Self::from_raw((*raw).next) }
>>> + }
>>> +
>>> + /// Get the previous [`CListHead`] in the list.
>>> + #[inline]
>>> + pub fn prev(&self) -> &Self {
>>> + let raw = self.as_raw();
>>> + // SAFETY:
>>> + // - self.as_raw() is valid per type invariants.
>>> + // - The `prev` pointer is guaranteed to be non-NULL.
>>> + unsafe { Self::from_raw((*raw).prev) }
>>> + }
>>> +
>>> + /// 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 }
>>
>> While is this checking both prev and next? `list_empty` is just
>> `READ_ONCE(head->next) == head`.
>
> Sure, I can optimize to just check ->next, that makes sense. Will do.
>
The important part is to make sure we don't deviate from C implementation. A
copy is already not good, and difference is worse.
>>
>>> + }
>>> +
>>> + /// Fallible pin-initializer that initializes and then calls user closure.
>>> + ///
>>> + /// Initializes the list head first, then passes `&CListHead` to the closure.
>>> + /// This hides the raw FFI pointer from the user.
>>> + pub fn try_init<E>(
>>> + init_func: impl FnOnce(&CListHead) -> Result<(), E>,
>>> + ) -> impl PinInit<Self, E> {
>>> + // SAFETY: init_list_head initializes the list_head to point to itself.
>>> + // After initialization, we create a reference to pass to the closure.
>>> + unsafe {
>>> + pin_init::pin_init_from_closure(move |slot: *mut Self| {
>>> + init_list_head(slot.cast());
>>> + // SAFETY: slot is now initialized, safe to create reference.
>>> + init_func(&*slot)
>>
>> Why is this callback necessary? The user can just create the list head and
>> then reference it later? I don't see what this specifically gains over just
>> doing
>>
>> fn new() -> impl PinInit<Self>;
>>
>> and have user-side
>>
>> list <- CListHead::new(),
>> _: {
>> do_want_ever(&list)
>> }
>
> The list initialization can fail, see the GPU buddy patch:
>
> // Create pin-initializer that initializes list and allocates blocks.
> let init = try_pin_init!(AllocatedBlocks {
> list <- CListHead::try_init(|list| {
> // Lock while allocating to serialize with concurrent frees.
> let guard = buddy_arc.lock();
>
> // SAFETY: guard provides exclusive access, list is initialized.
> to_result(unsafe {
> bindings::gpu_buddy_alloc_blocks(
> guard.as_raw(),
> params.start_range_address,
> params.end_range_address,
> params.size_bytes,
> params.min_block_size_bytes,
> list.as_raw(),
> params.buddy_flags.as_raw(),
> )
> })
> }),
> buddy: Arc::clone(&buddy_arc),
> flags: params.buddy_flags,
> });
The list initialization doesn't fail? It's the subsequent action you did that
failed.
You can put failing things in the `_: { ... }` block too.
>
>>
>>
>>> + })
>>> + }
>>> + }
>>> +}
>>> +
>>> +// SAFETY: [`CListHead`] can be sent to any thread.
>>> +unsafe impl Send for CListHead {}
>>> +
>>> +// SAFETY: [`CListHead`] can be shared among threads as it is not modified
>>> +// by non-Rust code per type invariants.
>>> +unsafe impl Sync for CListHead {}
>>> +
>>> +impl PartialEq for CListHead {
>>> + fn eq(&self, other: &Self) -> bool {
>>> + self.as_raw() == other.as_raw()
>>
>> Or just `core::ptr::eq(self, other)`
>
> Sure, will fix.
>
>>
>>> + }
>>> +}
>>> +
>>> +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` macro or similar).
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// [`CListHeadIter`] is iterating over an allocated, initialized and valid list.
>>> +struct CListHeadIter<'a> {
>>> + current_head: &'a CListHead,
>>> + list_head: &'a CListHead,
>>> +}
>>> +
>>> +impl<'a> Iterator for CListHeadIter<'a> {
>>> + type Item = &'a CListHead;
>>> +
>>> + #[inline]
>>> + fn next(&mut self) -> Option<Self::Item> {
>>> + // Advance to next node.
>>> + let next = self.current_head.next();
>>> +
>>> + // Check if we've circled back to the sentinel head.
>>> + if next == self.list_head {
>>> + None
>>> + } else {
>>> + self.current_head = next;
>>> + Some(self.current_head)
>>> + }
>>
>> I think this could match the C iterator behaviour. When the iterator is created,
>> a `next` is done first, and then subsequently you only need to check if
>> `current_head` is `list_head`.
>>
>> This is slightly better because the condition check does not need to dereference
>> a pointer.
>
> Sure, I can change it to that.
>>> +impl<'a> FusedIterator for CListHeadIter<'a> {}
>>> +
>>> +/// A typed C linked list with a sentinel head.
>>> +///
>>> +/// A sentinel head 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
>>> +///
>>> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
>>> +/// - `OFFSET` is the byte offset of the `list_head` field within the struct that `T` wraps.
>>> +/// - All the list's `list_head` nodes are allocated and have valid next/prev pointers.
>>> +/// - The underlying `list_head` (and entire list) is not modified for the lifetime `'a`.
>>> +pub struct CList<'a, T, const OFFSET: usize> {
>>> + head: &'a CListHead,
>>> + _phantom: PhantomData<&'a T>,
>>> +}
>>
>> Is there a reason that this is not
>>
>> #[repr(transparent)]
>> struct CList(CListHead)
>>
>> ? We typically want to avoid putting reference inside the struct if it can be on
>> the outside. This allows `&self` to be a single level of reference, not too.
>>
>> It also means that you can just write `&CList<_>` in many cases, and doesn't need
>> `CList<'_, T>` (plus all the benefits of a reference).
>
> Sure! Will change to this. I am guessing you mean the following, but please let
> me know if you meant something else:
>
> pub struct CList<T, const OFFSET: usize>(
> CListHead,
> PhantomData<T>,
> );
>
> I don't see any issues with my code using that, at the moment. Will let you know
> how it goes.
Yes, with `#[repr(transparent)]`.
>>> +impl<'a, T, const OFFSET: usize> CList<'a, T, OFFSET> {
>>> + /// Create a typed [`CList`] from a raw sentinel `list_head` pointer.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>>> + /// representing a list sentinel.
>>> + /// - `ptr` must remain valid and unmodified 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(ptr: *mut bindings::list_head) -> Self {
>>> + Self {
>>> + // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>>> + head: unsafe { CListHead::from_raw(ptr) },
>>> + _phantom: PhantomData,
>>> + }
>>> + }
>>> +
>>> + /// Get the raw sentinel `list_head` pointer.
>>> + #[inline]
>>> + pub fn as_raw(&self) -> *mut bindings::list_head {
>>> + self.head.as_raw()
>>> + }
>>> +
>>> + /// Check if the list is empty.
>>> + #[inline]
>>> + pub fn is_empty(&self) -> bool {
>>> + let raw = self.as_raw();
>>> + // SAFETY: self.as_raw() is valid per type invariants.
>>> + unsafe { (*raw).next == raw }
>>
>> `self.head.is_linked()`?
>
> I'd considered `is_linked()` to be something that makes sense to call only on
> `ClistHead` objects that belong to a particular "item" node, not a sentinel
> node, so that was deliberate.
>
> Though, I am Ok with doing it the way you are suggesting too
> (`self.head.is_linked()`), since it is functionally equivalent.
>
>>> + }
>>> +
>>> + /// Create an iterator over typed items.
>>> + #[inline]
>>> + pub fn iter(&self) -> CListIter<'a, T, OFFSET> {
>>> + CListIter {
>>> + head_iter: CListHeadIter {
>>> + current_head: self.head,
>>> + list_head: self.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;
>>> +
>>> + fn next(&mut self) -> Option<Self::Item> {
>>> + let head = self.head_iter.next()?;
>>> +
>>> + // Convert to item using OFFSET.
>>> + // SAFETY: `item_ptr` calculation from `OFFSET` (calculated using offset_of!)
>>> + // is valid per 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` pointing to a list that remains
>>> +/// unmodified for the lifetime of the rust [`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.
>>> +/// - The macro is called from an unsafe block.
>>
>> This is not a safe requirement, probably lift it up and say "This is an unsafe
>> macro.".
>
> Sure, so like this then:
> /// This is an unsafe macro. The caller must ensure:
> /// - `$head` is a valid, initialized sentinel `list_head`...
Yes.
Best,
Gary
>
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// Refer to the examples in the [`crate::clist`] 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| ::core::ptr::addr_of!((*p).$($field).+);
>>
>> `&raw const` is preferred now.
>
> Sure, will fix.
>
>>
>>> +
>>> + // Calculate offset and create `CList`.
>>> + const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>>> + $crate::clist::CList::<$rust_type, OFFSET>::from_raw($head)
>>> + }};
>>> +}
>>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>>> index f812cf120042..cd7e6a1055b0 100644
>>> --- a/rust/kernel/lib.rs
>>> +++ b/rust/kernel/lib.rs
>>> @@ -75,6 +75,7 @@
>>> pub mod bug;
>>> #[doc(hidden)]
>>> pub mod build_assert;
>>> +pub mod clist;
>>
>> Can we keep this pub(crate)?
>
> Yes, will do.
^ permalink raw reply
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-01-21 19:50 UTC (permalink / raw)
To: Gary Guo, linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <DFTTGUYGY72V.3VLVSCB2OOXIB@garyguo.net>
Hello, Gary,
On 1/20/2026 6:48 PM, Gary Guo wrote:
> On Tue Jan 20, 2026 at 8:42 PM GMT, Joel Fernandes wrote:
>> Add a new module `clist` 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.
>
> This should read "CList".
Sure, will fix.
>
> I was quite dubious about the patch just from the title (everybody knows how
> easy a linked list is in Rust), but it turns out it is not as concerning as I
> expected, mostly due to the read-only nature of the particular implementation
> (a lot of the safety comments would be much more difficult to justify, say, if
> it's mutable). That said, still a lot of feedbacks below.
Sure, the reason for requiring this is interfacing with lists coming from C
code. I'd see a future where we may want it mutable too (example, Rust code
adding elements to the existing). At which point, the invariants/safety
reasoning may change.
> I think something like is okay in the short term. However, there's an growing
> interest in getting our Rust list API improved, so it could be ideal if
> eventually the Rust list can be capable of handling FFI lists, too.
Yeah we looked into that, if you see old threads, the conclusion was it is not a
good fit for existing rust list abstractions. TLDR; it does not fit into their
ownership/borrowing model.
[...]
>> +
>> +/// Initialize a `list_head` object to point to itself.
>> +///
>> +/// # Safety
>> +///
>> +/// `list` must be a valid pointer to a `list_head` object.
>> +#[inline]
>> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
>> + // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
>> + unsafe {
>> + (*list).next = list;
>> + (*list).prev = list;
>
> This needs to be an atomic write or it'll depart from the C implementation.
I am curious what you mean by atomic write, can you define it? Does rust
compiler have load/store fusing, invented stores, etc, like C does? Sorry I am
only familiar with these concepts on C. Could you provide example of a race
condition in Rust that can happen?
Also I did this addition based on feedback from past review:
https://lore.kernel.org/all/DEI89VUEYXAJ.1IQQPC3QRLITP@nvidia.com/
There was some concerns around pointless function call overhead when the rust
implementation is already quite intertwined with internals of the C linked list
implementation. I do agree with that point of view too.
Also see my other reply to Zhi on this helper topic, lets discuss there too, if
that's Ok.
>> + }
>> +}
>
> I don't think we want to publicly expose this! I've not found a user in the
> subsequent patch, too.
There are 2 users:
pub fn try_init<E>(
and the self-tests:
//! # let head = head.as_mut_ptr();
//! # // SAFETY: head and all the items are test objects allocated in [..]
//! # unsafe { init_list_head(head) };
//! #
>
>> +
>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>> +///
>> +/// # Invariants
>> +///
>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>> +#[repr(transparent)]
>> +pub struct CListHead(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 allocated and initialized `list_head` structure.
>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>> + #[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.
>> + unsafe { &*ptr.cast() }
>> + }
>> +
>> + /// Get the raw `list_head` pointer.
>> + #[inline]
>> + pub fn as_raw(&self) -> *mut bindings::list_head {
>> + self.0.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 per type invariants.
>> + // - The `next` pointer is guaranteed to be non-NULL.
>> + unsafe { Self::from_raw((*raw).next) }
>> + }
>> +
>> + /// Get the previous [`CListHead`] in the list.
>> + #[inline]
>> + pub fn prev(&self) -> &Self {
>> + let raw = self.as_raw();
>> + // SAFETY:
>> + // - self.as_raw() is valid per type invariants.
>> + // - The `prev` pointer is guaranteed to be non-NULL.
>> + unsafe { Self::from_raw((*raw).prev) }
>> + }
>> +
>> + /// 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 }
>
> While is this checking both prev and next? `list_empty` is just
> `READ_ONCE(head->next) == head`.
Sure, I can optimize to just check ->next, that makes sense. Will do.
>
>> + }
>> +
>> + /// Fallible pin-initializer that initializes and then calls user closure.
>> + ///
>> + /// Initializes the list head first, then passes `&CListHead` to the closure.
>> + /// This hides the raw FFI pointer from the user.
>> + pub fn try_init<E>(
>> + init_func: impl FnOnce(&CListHead) -> Result<(), E>,
>> + ) -> impl PinInit<Self, E> {
>> + // SAFETY: init_list_head initializes the list_head to point to itself.
>> + // After initialization, we create a reference to pass to the closure.
>> + unsafe {
>> + pin_init::pin_init_from_closure(move |slot: *mut Self| {
>> + init_list_head(slot.cast());
>> + // SAFETY: slot is now initialized, safe to create reference.
>> + init_func(&*slot)
>
> Why is this callback necessary? The user can just create the list head and
> then reference it later? I don't see what this specifically gains over just
> doing
>
> fn new() -> impl PinInit<Self>;
>
> and have user-side
>
> list <- CListHead::new(),
> _: {
> do_want_ever(&list)
> }
The list initialization can fail, see the GPU buddy patch:
// Create pin-initializer that initializes list and allocates blocks.
let init = try_pin_init!(AllocatedBlocks {
list <- CListHead::try_init(|list| {
// Lock while allocating to serialize with concurrent frees.
let guard = buddy_arc.lock();
// SAFETY: guard provides exclusive access, list is initialized.
to_result(unsafe {
bindings::gpu_buddy_alloc_blocks(
guard.as_raw(),
params.start_range_address,
params.end_range_address,
params.size_bytes,
params.min_block_size_bytes,
list.as_raw(),
params.buddy_flags.as_raw(),
)
})
}),
buddy: Arc::clone(&buddy_arc),
flags: params.buddy_flags,
});
>
>
>> + })
>> + }
>> + }
>> +}
>> +
>> +// SAFETY: [`CListHead`] can be sent to any thread.
>> +unsafe impl Send for CListHead {}
>> +
>> +// SAFETY: [`CListHead`] can be shared among threads as it is not modified
>> +// by non-Rust code per type invariants.
>> +unsafe impl Sync for CListHead {}
>> +
>> +impl PartialEq for CListHead {
>> + fn eq(&self, other: &Self) -> bool {
>> + self.as_raw() == other.as_raw()
>
> Or just `core::ptr::eq(self, other)`
Sure, will fix.
>
>> + }
>> +}
>> +
>> +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` macro or similar).
>> +///
>> +/// # Invariants
>> +///
>> +/// [`CListHeadIter`] is iterating over an allocated, initialized and valid list.
>> +struct CListHeadIter<'a> {
>> + current_head: &'a CListHead,
>> + list_head: &'a CListHead,
>> +}
>> +
>> +impl<'a> Iterator for CListHeadIter<'a> {
>> + type Item = &'a CListHead;
>> +
>> + #[inline]
>> + fn next(&mut self) -> Option<Self::Item> {
>> + // Advance to next node.
>> + let next = self.current_head.next();
>> +
>> + // Check if we've circled back to the sentinel head.
>> + if next == self.list_head {
>> + None
>> + } else {
>> + self.current_head = next;
>> + Some(self.current_head)
>> + }
>
> I think this could match the C iterator behaviour. When the iterator is created,
> a `next` is done first, and then subsequently you only need to check if
> `current_head` is `list_head`.
>
> This is slightly better because the condition check does not need to dereference
> a pointer.
Sure, I can change it to that.
>> +impl<'a> FusedIterator for CListHeadIter<'a> {}
>> +
>> +/// A typed C linked list with a sentinel head.
>> +///
>> +/// A sentinel head 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
>> +///
>> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
>> +/// - `OFFSET` is the byte offset of the `list_head` field within the struct that `T` wraps.
>> +/// - All the list's `list_head` nodes are allocated and have valid next/prev pointers.
>> +/// - The underlying `list_head` (and entire list) is not modified for the lifetime `'a`.
>> +pub struct CList<'a, T, const OFFSET: usize> {
>> + head: &'a CListHead,
>> + _phantom: PhantomData<&'a T>,
>> +}
>
> Is there a reason that this is not
>
> #[repr(transparent)]
> struct CList(CListHead)
>
> ? We typically want to avoid putting reference inside the struct if it can be on
> the outside. This allows `&self` to be a single level of reference, not too.
>
> It also means that you can just write `&CList<_>` in many cases, and doesn't need
> `CList<'_, T>` (plus all the benefits of a reference).
Sure! Will change to this. I am guessing you mean the following, but please let
me know if you meant something else:
pub struct CList<T, const OFFSET: usize>(
CListHead,
PhantomData<T>,
);
I don't see any issues with my code using that, at the moment. Will let you know
how it goes.
>> +impl<'a, T, const OFFSET: usize> CList<'a, T, OFFSET> {
>> + /// Create a typed [`CList`] from a raw sentinel `list_head` pointer.
>> + ///
>> + /// # Safety
>> + ///
>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>> + /// representing a list sentinel.
>> + /// - `ptr` must remain valid and unmodified 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(ptr: *mut bindings::list_head) -> Self {
>> + Self {
>> + // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>> + head: unsafe { CListHead::from_raw(ptr) },
>> + _phantom: PhantomData,
>> + }
>> + }
>> +
>> + /// Get the raw sentinel `list_head` pointer.
>> + #[inline]
>> + pub fn as_raw(&self) -> *mut bindings::list_head {
>> + self.head.as_raw()
>> + }
>> +
>> + /// Check if the list is empty.
>> + #[inline]
>> + pub fn is_empty(&self) -> bool {
>> + let raw = self.as_raw();
>> + // SAFETY: self.as_raw() is valid per type invariants.
>> + unsafe { (*raw).next == raw }
>
> `self.head.is_linked()`?
I'd considered `is_linked()` to be something that makes sense to call only on
`ClistHead` objects that belong to a particular "item" node, not a sentinel
node, so that was deliberate.
Though, I am Ok with doing it the way you are suggesting too
(`self.head.is_linked()`), since it is functionally equivalent.
>> + }
>> +
>> + /// Create an iterator over typed items.
>> + #[inline]
>> + pub fn iter(&self) -> CListIter<'a, T, OFFSET> {
>> + CListIter {
>> + head_iter: CListHeadIter {
>> + current_head: self.head,
>> + list_head: self.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;
>> +
>> + fn next(&mut self) -> Option<Self::Item> {
>> + let head = self.head_iter.next()?;
>> +
>> + // Convert to item using OFFSET.
>> + // SAFETY: `item_ptr` calculation from `OFFSET` (calculated using offset_of!)
>> + // is valid per 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` pointing to a list that remains
>> +/// unmodified for the lifetime of the rust [`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.
>> +/// - The macro is called from an unsafe block.
>
> This is not a safe requirement, probably lift it up and say "This is an unsafe
> macro.".
Sure, so like this then:
/// This is an unsafe macro. The caller must ensure:
/// - `$head` is a valid, initialized sentinel `list_head`...
>> +///
>> +/// # Examples
>> +///
>> +/// Refer to the examples in the [`crate::clist`] 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| ::core::ptr::addr_of!((*p).$($field).+);
>
> `&raw const` is preferred now.
Sure, will fix.
>
>> +
>> + // Calculate offset and create `CList`.
>> + const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>> + $crate::clist::CList::<$rust_type, OFFSET>::from_raw($head)
>> + }};
>> +}
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index f812cf120042..cd7e6a1055b0 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -75,6 +75,7 @@
>> pub mod bug;
>> #[doc(hidden)]
>> pub mod build_assert;
>> +pub mod clist;
>
> Can we keep this pub(crate)?
Yes, will do.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 14/26] nova-core: mm: Add TLB flush support
From: Joel Fernandes @ 2026-01-21 18:45 UTC (permalink / raw)
To: Zhi Wang
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, 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, Danilo Krummrich, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Alistair Popple, Alexandre Courbot, Andrea Righi,
Alexey Ivanov, Philipp Stanner, Elle Rhumsaa, Daniel Almeida,
joel, nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260121115917.73cfcc7f.zhiw@nvidia.com>
Hello, Zhi,
On 1/21/2026 4:59 AM, Zhi Wang wrote:
> On Tue, 20 Jan 2026 15:42:51 -0500
> Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
>> Add TLB (Translation Lookaside Buffer) flush support for GPU MMU.
>>
> The same concern as in PATCH 5, guess we need to think of concurrency for
> TLB flush.
Will change:
pub(crate) fn flush(&self, pdb_addr: VramAddress)
to:
pub(crate) fn flush(&mut self, pdb_addr: VramAddress)
and also changing in mm/mod.rs:
pub(crate) fn tlb(&self) -> &Tlb {
to:
pub(crate) fn tlb(&mut self) -> &mut Tlb.
Since TLB operations modify registers, that does make sense to me.
For the buddy allocator, however, I am locking internally so I left it as is:
/// Access the [`GpuBuddy`] allocator.
pub(crate) fn buddy(&self) -> &GpuBuddy {
&self.buddy
}
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 13/26] nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-01-21 18:35 UTC (permalink / raw)
To: Zhi Wang
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, 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, Danilo Krummrich, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Alistair Popple, Alexandre Courbot, Andrea Righi,
Alexey Ivanov, Philipp Stanner, Elle Rhumsaa, Daniel Almeida,
joel, nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260121115436.45e174d3.zhiw@nvidia.com>
On 1/21/2026 4:54 AM, Zhi Wang wrote:
> On Tue, 20 Jan 2026 15:42:50 -0500
> Joel Fernandes <joelagnelf@nvidia.com> wrote:
>> Add unified Pte, Pde, and DualPde wrapper enums that abstract over
>> MMU v2 and v3 page table entry formats. These enums allow the page
>> table walker and VMM to work with both MMU versions.
>>
>
> snip
>
>> +impl DualPde {
>> + /// Create a [`DualPde`] from raw 128-bit value (two `u64`s) for
>> the given MMU version.
>> + pub(crate) fn new(version: MmuVersion, big: u64, small: u64) ->
>> Self {
>> + match version {
>> + MmuVersion::V2 => Self::V2(ver2::DualPde::new(big, small)),
>> + MmuVersion::V3 => Self::V3(ver3::DualPde::new(big, small)),
>> + }
>> + }
>> +
>> + /// Create a [`DualPde`] with only the small page table pointer set.
>> + pub(crate) fn new_small(version: MmuVersion, table_pfn: Pfn) ->
>> Self {
>> + match version {
>> + MmuVersion::V2 =>
>> Self::V2(ver2::DualPde::new_small(table_pfn)),
>> + MmuVersion::V3 =>
>> Self::V3(ver3::DualPde::new_small(table_pfn)),
>> + }
>> + }
>> +
>> + /// Check if the small page table pointer is valid.
>> + pub(crate) fn has_small(&self) -> bool {
>> + match self {
>> + Self::V2(d) => d.has_small(),
>> + Self::V3(d) => d.has_small(),
>> + }
>> + }
>> +
>
> Should we also have a has_big here as well?
Good catch, I will add that in, thanks.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-01-21 18:12 UTC (permalink / raw)
To: Zhi Wang, Joel Fernandes
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, nouveau, dri-devel, rust-for-linux,
linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev, Boqun Feng,
Paul E. McKenney
In-Reply-To: <20260121092730.3628d290.zhiw@nvidia.com>
On 1/21/2026 2:27 AM, Zhi Wang wrote:
>
>> +/// Initialize a `list_head` object to point to itself.
>> +///
>> +/// # Safety
>> +///
>> +/// `list` must be a valid pointer to a `list_head` object.
>> +#[inline]
>> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
>> + // SAFETY: Caller guarantees `list` is a valid pointer to a
>> `list_head`.
>> + unsafe {
>> + (*list).next = list;
>> + (*list).prev = list;
>> + }
>> +}
>> +
>
> Might be better to have a C helper? since INIT_LIST_HEAD() has WRITE_ONCE()
> for memory ordering. This one seems not equal to it.
WRITE_ONCE() is not really about CPU memory ordering though, it is about
compiler optimizations. On the C side, I think it is needed in case of
list_for_each_entry_rcu(), to avoid the case of invented stores or store fusing,
but here we are not doing RCU-based iteration.
Anyway, if we want to future proof that, I am Ok with adding the helper back
(which I actually initially had but feedback from past review was to just inline
it into rust).
But I am not sure if we have this issue for the Rust compiler, like we do for C.
Rust does not allow raw pointers to be concurrently read/written using plain
accesses, so should be already protected due to the borrow checker and compiler
itself right?
Adding some interested folks as well to CC for the topic of _ONCE, +Boqun +Paul.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 05/26] nova-core: mm: Add support to use PRAMIN windows to write to VRAM
From: Joel Fernandes @ 2026-01-21 17:52 UTC (permalink / raw)
To: Zhi Wang
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <20260121100745.2b5a58e5.zhiw@nvidia.com>
Hello, Zhi,
On 1/21/2026 3:07 AM, Zhi Wang wrote:
> On Tue, 20 Jan 2026 15:42:42 -0500
> Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
>> PRAMIN apertures are a crucial mechanism to direct read/write to VRAM.
>> Add support for the same.
>>
>
> I went through the code, this seems not designed for multiple users. As
> this is used for writting PTEs for page tables, can you shed some light
> about the plan of how we should handle the concurrency of writting multiple
> page table PTEs, e.g. when two GPU memory mapping in two different GPU
> page tables are procceding concurrently, this could happen when people
> creating vGPUs concurrently.
Good question. Currently, BarUser::map() requires a mutable reference to both
the BarUser and the GpuMm.
pub(crate) fn map<'a>(
&'a mut self,
mm: &'a mut GpuMm,
GpuMm is owned by the struct Gpu, so from a Rust standpoint, it is already
handled since it is not possible to manipulate the Page table hierarchy (Page
directories and last level Page table).
But yes, we have to look into concurrency once we have channels, and users other
than bar where have multiple users of the same address space doing
mapping/unmapping.
I think we can incrementally build on this series to add support for the same,
it is not something this series directly addresses since I have spend majority
of my time last several months making translation *work* which is itself no east
task. This series is just preliminary based on work from last several months and
to make BAR1 work. For instance, I kept PRAMIN simple based on feedback that we
don't want to over complicate without fully understanding all the requirements.
There is also additional requirements for locking design that have implications
with DMA fencing etc, for instance.
Anyway thinking out loud, I am thinking for handling concurrency at the page
table entry level (if we ever need it), we could use per-PT spinlocks similar to
the Linux kernel. But lets plan on how to do this properly and based on actual
requirements.
--
Joel Fernandes
^ permalink raw reply
* Re: printk's threaded legacy console + fbcon => schedule where it should not
From: Petr Mladek @ 2026-01-21 16:08 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Steven Rostedt, linux-kernel, linux-serial, linux-fbdev,
John Ogness, Sergey Senozhatsky, Greg Kroah-Hartman, Jiri Slaby,
Simona Vetter, Helge Deller
In-Reply-To: <20260121135737.K7b-4M5r@linutronix.de>
On Wed 2026-01-21 14:57:37, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 14:43:45 [+0100], Petr Mladek wrote:
> > I know that there was a plan to get rid of cond_resched().
> > But what is the status now, please?
>
> It is slowly moving => https://lore.kernel.org/all/20251219101502.GB1132199@noisy.programming.kicks-ass.net/
Good to know.
> > I still see more that 1k cond_resched() calls in the code:
> >
> > $> git grep cond_resched\(\) | grep "\.c:" | wc -l
> > 1263
> >
> > And config PREEMPT_VOLUNTARY still talks about the explicit
> > preemption points.
> >
> > > Should we just remove it and see what breaks?
> >
> > Honestly, I do not feel comfortable with removing it. It is true that
> > it has no effect in the printk() code path. But the vt code is used
> > also when working on the terminal.
> >
> > The vt code still uses console_lock() because it was intertwined
> > with printk console code since very old days. console_lock is a kind
> > of big kernel lock there.
>
> Do you a have path which loops and would mandate it? I found a few which
> do not matter and have their own cond_resched() around. So I don't see a
> reason to keep it. And I found one which breaks things so a removal
> makes sense.
Could anyone from VT guys comment on it, please?
> > Alternative solution is to get rid of the spin_trylock(). The only
> > purpose is to prevent race in console_flush_on_panic(). It used
> > to be a simple bit operation. The spin_lock() was added just to
> > get barriers right. But we have a great atomic_t API these days.
> >
> > IMHO, it is a win-win solution because a preemptive context is
> > always better.
>
> So why do we keep warts again? Just because it *might* be required?
> Keeping things preemptible makes sense but this is locking with no
> annotation what so ever.
Well, the current locking is documented but it creates false
positives. The "printing_lock" is taken on a single place
using spin_trylock(). Nobody would ever spin on it. So
sleeping is perfectly fine.
> Again. printk has its cond_resched, the tty has it, too.
> I'm with Steven on the removal side.
As I said, the cond_resched() does not have any effect from
the printk() code path. But the other VT paths might rely on it.
If VT-guys are willing to take the risk and remove it
then I am fine with it.
Best Regards,
Petr
^ permalink raw reply
* Re: printk's threaded legacy console + fbcon => schedule where it should not
From: Sebastian Andrzej Siewior @ 2026-01-21 13:57 UTC (permalink / raw)
To: Petr Mladek
Cc: Steven Rostedt, linux-kernel, linux-serial, linux-fbdev,
John Ogness, Sergey Senozhatsky, Greg Kroah-Hartman, Jiri Slaby,
Simona Vetter, Helge Deller
In-Reply-To: <aXDYEVlkFgxdSVSG@pathway.suse.cz>
On 2026-01-21 14:43:45 [+0100], Petr Mladek wrote:
> I know that there was a plan to get rid of cond_resched().
> But what is the status now, please?
It is slowly moving => https://lore.kernel.org/all/20251219101502.GB1132199@noisy.programming.kicks-ass.net/
> I still see more that 1k cond_resched() calls in the code:
>
> $> git grep cond_resched\(\) | grep "\.c:" | wc -l
> 1263
>
> And config PREEMPT_VOLUNTARY still talks about the explicit
> preemption points.
>
> > Should we just remove it and see what breaks?
>
> Honestly, I do not feel comfortable with removing it. It is true that
> it has no effect in the printk() code path. But the vt code is used
> also when working on the terminal.
>
> The vt code still uses console_lock() because it was intertwined
> with printk console code since very old days. console_lock is a kind
> of big kernel lock there.
Do you a have path which loops and would mandate it? I found a few which
do not matter and have their own cond_resched() around. So I don't see a
reason to keep it. And I found one which breaks things so a removal
makes sense.
> Alternative solution is to get rid of the spin_trylock(). The only
> purpose is to prevent race in console_flush_on_panic(). It used
> to be a simple bit operation. The spin_lock() was added just to
> get barriers right. But we have a great atomic_t API these days.
>
> IMHO, it is a win-win solution because a preemptive context is
> always better.
So why do we keep warts again? Just because it *might* be required?
Keeping things preemptible makes sense but this is locking with no
annotation what so ever.
Again. printk has its cond_resched, the tty has it, too.
I'm with Steven on the removal side.
Sebastian
^ permalink raw reply
* Re: printk's threaded legacy console + fbcon => schedule where it should not
From: Petr Mladek @ 2026-01-21 13:43 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sebastian Andrzej Siewior, linux-kernel, linux-serial,
linux-fbdev, John Ogness, Sergey Senozhatsky, Greg Kroah-Hartman,
Jiri Slaby, Simona Vetter, Helge Deller
In-Reply-To: <20260120110845.2922a91a@gandalf.local.home>
On Tue 2026-01-20 11:08:45, Steven Rostedt wrote:
> On Wed, 14 Jan 2026 15:59:55 +0100
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
> > @@ -3362,22 +3362,6 @@ void console_unlock(void)
> > }
> > EXPORT_SYMBOL(console_unlock);
> >
> > -/**
> > - * console_conditional_schedule - yield the CPU if required
>
> Egad! That goes all the way back to 2002:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=a880f45a48be2956d2c78a839c472287d54435c1
>
> > - *
> > - * If the console code is currently allowed to sleep, and
> > - * if this CPU should yield the CPU to another task, do
> > - * so here.
> > - *
> > - * Must be called within console_lock();.
> > - */
> > -void __sched console_conditional_schedule(void)
> > -{
> > - if (console_may_schedule)
> > - cond_resched();
> > -}
> > -EXPORT_SYMBOL(console_conditional_schedule);
>
> I'm assuming this likely isn't needed anymore. I don't know of any reason
> it needs to stay.
I know that there was a plan to get rid of cond_resched().
But what is the status now, please?
I still see more that 1k cond_resched() calls in the code:
$> git grep cond_resched\(\) | grep "\.c:" | wc -l
1263
And config PREEMPT_VOLUNTARY still talks about the explicit
preemption points.
> Should we just remove it and see what breaks?
Honestly, I do not feel comfortable with removing it. It is true that
it has no effect in the printk() code path. But the vt code is used
also when working on the terminal.
The vt code still uses console_lock() because it was intertwined
with printk console code since very old days. console_lock is a kind
of big kernel lock there.
Alternative solution is to get rid of the spin_trylock(). The only
purpose is to prevent race in console_flush_on_panic(). It used
to be a simple bit operation. The spin_lock() was added just to
get barriers right. But we have a great atomic_t API these days.
IMHO, it is a win-win solution because a preemptive context is
always better.
What about?
From 0fc61b6877e9beb20429effc599bc4bc6ec3a475 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Wed, 21 Jan 2026 10:47:15 +0100
Subject: [RFC] tty/vt: Prevent re-entering vt_console_print() in panic()
without spin_lock
The commit b0940003f25dd ("vt: bitlock fix") replaced a simple bit
operation with spin_lock() to get proper memory barriers.
But the code called under this lock calls console_conditional_schedule()
which calls cond_resched() when console_sem() has been acquired
in a preemptive context using console_lock(). Note that the semaphore
can be taken also in an atomic context using console_trylock()
which is used by printk().
One solution would be to remove console_conditional_schedule().
It does not have any effect in the printk() code path anyway.
But the affected VT code is not used just by printk(). And
the cond_resched() calls were likely added for a reason.
Instead, convert the spin_lock back to an atomic operation with
proper barriers. The only purpose of the lock is to prevent
a concurrent access to the guarded code in
console_flush_on_panic() where console_lock() is ignored.
Using a full featured spin_trylock, just to get memory barriers
right, looks like an overkill anyway.
Fixes: b0940003f25dd ("vt: bitlock fix")
Closes: https://lore.kernel.org/all/20260114145955.d924Z-zu@linutronix.de/
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
drivers/tty/vt/vt.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 59b4b5e126ba..5be64d1bba91 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3353,15 +3353,19 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
{
struct vc_data *vc = vc_cons[fg_console].d;
unsigned char c;
- static DEFINE_SPINLOCK(printing_lock);
+ static atomic_t printing_lock = ATOMIC_INIT(0);
const ushort *start;
ushort start_x, cnt;
int kmsg_console;
WARN_CONSOLE_UNLOCKED();
- /* this protects against concurrent oops only */
- if (!spin_trylock(&printing_lock))
+ /*
+ * Prevent concurrent printing in console_flush_on_panic() where
+ * console_lock is ignored. Easier (serial) console drivers
+ * have bigger chance to get the messages out.
+ */
+ if (atomic_cmpxchg_acquire(&printing_lock, 0, 1) != 0)
return;
kmsg_console = vt_get_kmsg_redirect();
@@ -3422,7 +3426,7 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
notify_update(vc);
quit:
- spin_unlock(&printing_lock);
+ atomic_set_release(&printing_lock, 0);
}
static struct tty_driver *vt_console_device(struct console *c, int *index)
--
2.52.0
Best Regards,
Petr
^ permalink raw reply related
* Re: [PATCH RFC v6 14/26] nova-core: mm: Add TLB flush support
From: Zhi Wang @ 2026-01-21 9:59 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, 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, Danilo Krummrich, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Alistair Popple, Alexandre Courbot, Andrea Righi,
Alexey Ivanov, Philipp Stanner, Elle Rhumsaa, Daniel Almeida,
joel, nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260120204303.3229303-15-joelagnelf@nvidia.com>
On Tue, 20 Jan 2026 15:42:51 -0500
Joel Fernandes <joelagnelf@nvidia.com> wrote:
> Add TLB (Translation Lookaside Buffer) flush support for GPU MMU.
>
The same concern as in PATCH 5, guess we need to think of concurrency for
TLB flush.
> After modifying page table entries, the GPU's TLB must be invalidated
> to ensure the new mappings take effect. The Tlb struct provides flush
> functionality through BAR0 registers.
>
> The flush operation writes the page directory base address and triggers
> an invalidation, polling for completion with a 2 second timeout matching
> the Nouveau driver.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/mm/mod.rs | 1 +
> drivers/gpu/nova-core/mm/tlb.rs | 79 +++++++++++++++++++++++++++++++++
> drivers/gpu/nova-core/regs.rs | 33 ++++++++++++++
> 3 files changed, 113 insertions(+)
> create mode 100644 drivers/gpu/nova-core/mm/tlb.rs
>
> diff --git a/drivers/gpu/nova-core/mm/mod.rs
> b/drivers/gpu/nova-core/mm/mod.rs index 6015fc8753bc..39635f2d0156 100644
> --- a/drivers/gpu/nova-core/mm/mod.rs
> +++ b/drivers/gpu/nova-core/mm/mod.rs
> @@ -6,6 +6,7 @@
>
> pub(crate) mod pagetable;
> pub(crate) mod pramin;
> +pub(crate) mod tlb;
>
> use kernel::sizes::SZ_4K;
>
> diff --git a/drivers/gpu/nova-core/mm/tlb.rs
> b/drivers/gpu/nova-core/mm/tlb.rs new file mode 100644
> index 000000000000..8b2ee620da18
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/tlb.rs
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! TLB (Translation Lookaside Buffer) flush support for GPU MMU.
> +//!
> +//! After modifying page table entries, the GPU's TLB must be flushed to
> +//! ensure the new mappings take effect. This module provides TLB flush
> +//! functionality for virtual memory managers.
> +//!
> +//! # Example
> +//!
> +//! ```ignore
> +//! use crate::mm::tlb::Tlb;
> +//!
> +//! fn page_table_update(tlb: &Tlb, pdb_addr: VramAddress) ->
> Result<()> { +//! // ... modify page tables ...
> +//!
> +//! // Flush TLB to make changes visible (polls for completion).
> +//! tlb.flush(pdb_addr)?;
> +//!
> +//! Ok(())
> +//! }
> +//! ```
> +
> +#![allow(dead_code)]
> +
> +use kernel::{
> + devres::Devres,
> + io::poll::read_poll_timeout,
> + prelude::*,
> + sync::Arc,
> + time::Delta, //
> +};
> +
> +use crate::{
> + driver::Bar0,
> + mm::VramAddress,
> + regs, //
> +};
> +
> +/// TLB manager for GPU translation buffer operations.
> +pub(crate) struct Tlb {
> + bar: Arc<Devres<Bar0>>,
> +}
> +
> +impl Tlb {
> + /// Create a new TLB manager.
> + pub(super) fn new(bar: Arc<Devres<Bar0>>) -> Self {
> + Self { bar }
> + }
> +
> + /// Flush the GPU TLB for a specific page directory base.
> + ///
> + /// This invalidates all TLB entries associated with the given PDB
> address.
> + /// Must be called after modifying page table entries to ensure the
> GPU sees
> + /// the updated mappings.
> + pub(crate) fn flush(&self, pdb_addr: VramAddress) -> Result {
> + let bar = self.bar.try_access().ok_or(ENODEV)?;
> +
> + // Write PDB address.
> +
> regs::NV_TLB_FLUSH_PDB_LO::from_pdb_addr(pdb_addr.raw_u64()).write(&*bar);
> +
> regs::NV_TLB_FLUSH_PDB_HI::from_pdb_addr(pdb_addr.raw_u64()).write(&*bar);
> +
> + // Trigger flush: invalidate all pages and enable.
> + regs::NV_TLB_FLUSH_CTRL::default()
> + .set_page_all(true)
> + .set_enable(true)
> + .write(&*bar);
> +
> + // Poll for completion - enable bit clears when flush is done.
> + read_poll_timeout(
> + || Ok(regs::NV_TLB_FLUSH_CTRL::read(&*bar)),
> + |ctrl| !ctrl.enable(),
> + Delta::ZERO,
> + Delta::from_secs(2),
> + )?;
> +
> + Ok(())
> + }
> +}
> diff --git a/drivers/gpu/nova-core/regs.rs
> b/drivers/gpu/nova-core/regs.rs index c8b8fbdcf608..e722ef837e11 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -414,3 +414,36 @@ pub(crate) mod ga100 {
> 0:0 display_disabled as bool;
> });
> }
> +
> +// MMU TLB
> +
> +register!(NV_TLB_FLUSH_PDB_LO @ 0x00b830a0, "TLB flush register: PDB
> address bits [39:8]" {
> + 31:0 pdb_lo as u32, "PDB address bits [39:8]";
> +});
> +
> +impl NV_TLB_FLUSH_PDB_LO {
> + /// Create a register value from a PDB address.
> + ///
> + /// Extracts bits [39:8] of the address and shifts it right by 8
> bits.
> + pub(crate) fn from_pdb_addr(addr: u64) -> Self {
> + Self::default().set_pdb_lo(((addr >> 8) & 0xFFFF_FFFF) as u32)
> + }
> +}
> +
> +register!(NV_TLB_FLUSH_PDB_HI @ 0x00b830a4, "TLB flush register: PDB
> address bits [47:40]" {
> + 7:0 pdb_hi as u8, "PDB address bits [47:40]";
> +});
> +
> +impl NV_TLB_FLUSH_PDB_HI {
> + /// Create a register value from a PDB address.
> + ///
> + /// Extracts bits [47:40] of the address and shifts it right by 40
> bits.
> + pub(crate) fn from_pdb_addr(addr: u64) -> Self {
> + Self::default().set_pdb_hi(((addr >> 40) & 0xFF) as u8)
> + }
> +}
> +
> +register!(NV_TLB_FLUSH_CTRL @ 0x00b830b0, "TLB flush control register" {
> + 0:0 page_all as bool, "Invalidate all pages";
> + 31:31 enable as bool, "Enable/trigger flush (clears when flush
> completes)"; +});
^ permalink raw reply
* Re: [PATCH RFC v6 13/26] nova-core: mm: Add unified page table entry wrapper enums
From: Zhi Wang @ 2026-01-21 9:54 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, 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, Danilo Krummrich, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Alistair Popple, Alexandre Courbot, Andrea Righi,
Alexey Ivanov, Philipp Stanner, Elle Rhumsaa, Daniel Almeida,
joel, nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260120204303.3229303-14-joelagnelf@nvidia.com>
On Tue, 20 Jan 2026 15:42:50 -0500
Joel Fernandes <joelagnelf@nvidia.com> wrote:
> Add unified Pte, Pde, and DualPde wrapper enums that abstract over
> MMU v2 and v3 page table entry formats. These enums allow the page
> table walker and VMM to work with both MMU versions.
>
snip
> +impl DualPde {
> + /// Create a [`DualPde`] from raw 128-bit value (two `u64`s) for
> the given MMU version.
> + pub(crate) fn new(version: MmuVersion, big: u64, small: u64) ->
> Self {
> + match version {
> + MmuVersion::V2 => Self::V2(ver2::DualPde::new(big, small)),
> + MmuVersion::V3 => Self::V3(ver3::DualPde::new(big, small)),
> + }
> + }
> +
> + /// Create a [`DualPde`] with only the small page table pointer set.
> + pub(crate) fn new_small(version: MmuVersion, table_pfn: Pfn) ->
> Self {
> + match version {
> + MmuVersion::V2 =>
> Self::V2(ver2::DualPde::new_small(table_pfn)),
> + MmuVersion::V3 =>
> Self::V3(ver3::DualPde::new_small(table_pfn)),
> + }
> + }
> +
> + /// Check if the small page table pointer is valid.
> + pub(crate) fn has_small(&self) -> bool {
> + match self {
> + Self::V2(d) => d.has_small(),
> + Self::V3(d) => d.has_small(),
> + }
> + }
> +
Should we also have a has_big here as well?
Z.
> + /// Get the small page table VRAM address.
> + pub(crate) fn small_vram_address(&self) -> VramAddress {
> + match self {
> + Self::V2(d) => d.small.table_vram_address(),
> + Self::V3(d) => d.small.table_vram_address(),
> + }
> + }
> +
> + /// Get the raw `u64` value of the big PDE.
> + pub(crate) fn big_raw_u64(&self) -> u64 {
> + match self {
> + Self::V2(d) => d.big.raw_u64(),
> + Self::V3(d) => d.big.raw_u64(),
> + }
> + }
> +
> + /// Get the raw `u64` value of the small PDE.
> + pub(crate) fn small_raw_u64(&self) -> u64 {
> + match self {
> + Self::V2(d) => d.small.raw_u64(),
> + Self::V3(d) => d.small.raw_u64(),
> + }
> + }
> +}
^ permalink raw reply
* Re: [PATCH RFC v6 05/26] nova-core: mm: Add support to use PRAMIN windows to write to VRAM
From: Zhi Wang @ 2026-01-21 8:07 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <20260120204303.3229303-6-joelagnelf@nvidia.com>
On Tue, 20 Jan 2026 15:42:42 -0500
Joel Fernandes <joelagnelf@nvidia.com> wrote:
> PRAMIN apertures are a crucial mechanism to direct read/write to VRAM.
> Add support for the same.
>
I went through the code, this seems not designed for multiple users. As
this is used for writting PTEs for page tables, can you shed some light
about the plan of how we should handle the concurrency of writting multiple
page table PTEs, e.g. when two GPU memory mapping in two different GPU
page tables are procceding concurrently, this could happen when people
creating vGPUs concurrently.
Z.
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/mm/mod.rs | 5 +
> drivers/gpu/nova-core/mm/pramin.rs | 244 +++++++++++++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs | 1 +
> drivers/gpu/nova-core/regs.rs | 5 +
> 4 files changed, 255 insertions(+)
> create mode 100644 drivers/gpu/nova-core/mm/mod.rs
> create mode 100644 drivers/gpu/nova-core/mm/pramin.rs
>
> diff --git a/drivers/gpu/nova-core/mm/mod.rs
> b/drivers/gpu/nova-core/mm/mod.rs new file mode 100644
> index 000000000000..7a5dd4220c67
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/mod.rs
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Memory management subsystems for nova-core.
> +
> +pub(crate) mod pramin;
> diff --git a/drivers/gpu/nova-core/mm/pramin.rs
> b/drivers/gpu/nova-core/mm/pramin.rs new file mode 100644
> index 000000000000..6a7ea2dc7d77
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/pramin.rs
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Direct VRAM access through the PRAMIN aperture.
> +//!
> +//! PRAMIN provides a 1MB sliding window into VRAM through BAR0,
> allowing the CPU to access +//! video memory directly. The [`Window`]
> type automatically repositions the window when +//! accessing different
> VRAM regions and restores the original position on drop. This allows
> +//! to reuse the same window for multiple accesses in the same window.
> +//! +//! The PRAMIN aperture is a 1MB region at BAR0 + 0x700000 for all
> GPUs. The window base is +//! controlled by the `NV_PBUS_BAR0_WINDOW`
> register and must be 64KB aligned. +//!
> +//! # Examples
> +//!
> +//! ## Basic read/write
> +//!
> +//! ```no_run
> +//! use crate::driver::Bar0;
> +//! use crate::mm::pramin;
> +//! use kernel::devres::Devres;
> +//! use kernel::sync::Arc;
> +//!
> +//! fn example(devres_bar: Arc<Devres<Bar0>>) -> Result<()> {
> +//! let mut pram_win = pramin::Window::new(devres_bar)?;
> +//!
> +//! // Write and read back.
> +//! pram_win.try_write32(0x100, 0xDEADBEEF)?;
> +//! let val = pram_win.try_read32(0x100)?;
> +//! assert_eq!(val, 0xDEADBEEF);
> +//!
> +//! Ok(())
> +//! // Original window position restored on drop.
> +//! }
> +//! ```
> +//!
> +//! ## Auto-repositioning across VRAM regions
> +//!
> +//! ```no_run
> +//! use crate::driver::Bar0;
> +//! use crate::mm::pramin;
> +//! use kernel::devres::Devres;
> +//! use kernel::sync::Arc;
> +//!
> +//! fn example(devres_bar: Arc<Devres<Bar0>>) -> Result<()> {
> +//! let mut pram_win = pramin::Window::new(devres_bar)?;
> +//!
> +//! // Access first 1MB region.
> +//! pram_win.try_write32(0x100, 0x11111111)?;
> +//!
> +//! // Access at 2MB - window auto-repositions.
> +//! pram_win.try_write32(0x200000, 0x22222222)?;
> +//!
> +//! // Back to first region - window repositions again.
> +//! let val = pram_win.try_read32(0x100)?;
> +//! assert_eq!(val, 0x11111111);
> +//!
> +//! Ok(())
> +//! }
> +//! ```
> +
> +#![allow(unused)]
> +
> +use crate::{
> + driver::Bar0,
> + regs, //
> +};
> +
> +use kernel::bits::genmask_u64;
> +use kernel::devres::Devres;
> +use kernel::prelude::*;
> +use kernel::ptr::{
> + Alignable,
> + Alignment, //
> +};
> +use kernel::sizes::{
> + SZ_1M,
> + SZ_64K, //
> +};
> +use kernel::sync::Arc;
> +
> +/// PRAMIN aperture base offset in BAR0.
> +const PRAMIN_BASE: usize = 0x700000;
> +
> +/// PRAMIN aperture size (1MB).
> +const PRAMIN_SIZE: usize = SZ_1M;
> +
> +/// 64KB alignment for window base.
> +const WINDOW_ALIGN: Alignment = Alignment::new::<SZ_64K>();
> +
> +/// Maximum addressable VRAM offset (40-bit address space).
> +///
> +/// The `NV_PBUS_BAR0_WINDOW` register has a 24-bit `window_base` field
> (bits 23:0) that stores +/// bits [39:16] of the target VRAM address.
> This limits the addressable space to 2^40 bytes. +///
> +/// CAST: On 64-bit systems, this fits in usize.
> +const MAX_VRAM_OFFSET: usize = genmask_u64(0..=39) as usize;
> +
> +/// Generate a PRAMIN read accessor.
> +macro_rules! define_pramin_read {
> + ($name:ident, $ty:ty) => {
> + #[doc = concat!("Read a `", stringify!($ty), "` from VRAM at
> the given offset.")]
> + pub(crate) fn $name(&mut self, vram_offset: usize) ->
> Result<$ty> {
> + // Compute window parameters without bar reference.
> + let (bar_offset, new_base) =
> + self.compute_window(vram_offset,
> ::core::mem::size_of::<$ty>())?; +
> + // Update window base if needed and perform read.
> + let bar = self.bar.try_access().ok_or(ENODEV)?;
> + if let Some(base) = new_base {
> + Self::write_window_base(&bar, base);
> + self.current_base = base;
> + }
> + bar.$name(bar_offset)
> + }
> + };
> +}
> +
> +/// Generate a PRAMIN write accessor.
> +macro_rules! define_pramin_write {
> + ($name:ident, $ty:ty) => {
> + #[doc = concat!("Write a `", stringify!($ty), "` to VRAM at the
> given offset.")]
> + pub(crate) fn $name(&mut self, vram_offset: usize, value: $ty)
> -> Result {
> + // Compute window parameters without bar reference.
> + let (bar_offset, new_base) =
> + self.compute_window(vram_offset,
> ::core::mem::size_of::<$ty>())?; +
> + // Update window base if needed and perform write.
> + let bar = self.bar.try_access().ok_or(ENODEV)?;
> + if let Some(base) = new_base {
> + Self::write_window_base(&bar, base);
> + self.current_base = base;
> + }
> + bar.$name(value, bar_offset)
> + }
> + };
> +}
> +
> +/// PRAMIN window for direct VRAM access.
> +///
> +/// The window auto-repositions when accessing VRAM offsets outside the
> current 1MB range. +/// Original window position is saved on creation
> and restored on drop. +pub(crate) struct Window {
> + bar: Arc<Devres<Bar0>>,
> + saved_base: usize,
> + current_base: usize,
> +}
> +
> +impl Window {
> + /// Create a new PRAMIN window accessor.
> + ///
> + /// Saves the current window position for restoration on drop.
> + pub(crate) fn new(bar: Arc<Devres<Bar0>>) -> Result<Self> {
> + let bar_access = bar.try_access().ok_or(ENODEV)?;
> + let saved_base = Self::try_read_window_base(&bar_access)?;
> +
> + Ok(Self {
> + bar,
> + saved_base,
> + current_base: saved_base,
> + })
> + }
> +
> + /// Read the current window base from the BAR0_WINDOW register.
> + fn try_read_window_base(bar: &Bar0) -> Result<usize> {
> + let reg = regs::NV_PBUS_BAR0_WINDOW::read(bar);
> + let base = u64::from(reg.window_base());
> + let shifted = base.checked_shl(16).ok_or(EOVERFLOW)?;
> + shifted.try_into().map_err(|_| EOVERFLOW)
> + }
> +
> + /// Write a new window base to the BAR0_WINDOW register.
> + fn write_window_base(bar: &Bar0, base: usize) {
> + // CAST:
> + // - We have guaranteed that the base is within the addressable
> range (40-bits).
> + // - After >> 16, a 40-bit aligned base becomes 24 bits, which
> fits in u32.
> + regs::NV_PBUS_BAR0_WINDOW::default()
> + .set_window_base((base >> 16) as u32)
> + .write(bar);
> + }
> +
> + /// Compute window parameters for a VRAM access.
> + ///
> + /// Returns (bar_offset, new_base) where:
> + /// - bar_offset: The BAR0 offset to use for the access
> + /// - new_base: Some(base) if window needs repositioning, None
> otherwise
> + fn compute_window(
> + &self,
> + vram_offset: usize,
> + access_size: usize,
> + ) -> Result<(usize, Option<usize>)> {
> + // Validate VRAM offset is within addressable range (40-bit
> address space).
> + let end_offset =
> vram_offset.checked_add(access_size).ok_or(EINVAL)?;
> + if end_offset > MAX_VRAM_OFFSET + 1 {
> + return Err(EINVAL);
> + }
> +
> + // Calculate which 64KB-aligned base we need.
> + let needed_base = vram_offset.align_down(WINDOW_ALIGN);
> +
> + // Calculate offset within the window.
> + let offset_in_window = vram_offset - needed_base;
> +
> + // Check if access fits in 1MB window from this base.
> + if offset_in_window + access_size > PRAMIN_SIZE {
> + return Err(EINVAL);
> + }
> +
> + // Return bar offset and whether window needs repositioning.
> + let new_base = if self.current_base != needed_base {
> + Some(needed_base)
> + } else {
> + None
> + };
> +
> + Ok((PRAMIN_BASE + offset_in_window, new_base))
> + }
> +
> + define_pramin_read!(try_read8, u8);
> + define_pramin_read!(try_read16, u16);
> + define_pramin_read!(try_read32, u32);
> + define_pramin_read!(try_read64, u64);
> +
> + define_pramin_write!(try_write8, u8);
> + define_pramin_write!(try_write16, u16);
> + define_pramin_write!(try_write32, u32);
> + define_pramin_write!(try_write64, u64);
> +}
> +
> +impl Drop for Window {
> + fn drop(&mut self) {
> + // Restore the original window base if it changed.
> + if self.current_base != self.saved_base {
> + if let Some(bar) = self.bar.try_access() {
> + Self::write_window_base(&bar, self.saved_base);
> + }
> + }
> + }
> +}
> +
> +// SAFETY: `Window` requires `&mut self` for all accessors.
> +unsafe impl Send for Window {}
> +
> +// SAFETY: `Window` requires `&mut self` for all accessors.
> +unsafe impl Sync for Window {}
> diff --git a/drivers/gpu/nova-core/nova_core.rs
> b/drivers/gpu/nova-core/nova_core.rs index c1121e7c64c5..3de00db3279e
> 100644 --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -13,6 +13,7 @@
> mod gfw;
> mod gpu;
> mod gsp;
> +mod mm;
> mod num;
> mod regs;
> mod sbuffer;
> diff --git a/drivers/gpu/nova-core/regs.rs
> b/drivers/gpu/nova-core/regs.rs index 82cc6c0790e5..c8b8fbdcf608 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -96,6 +96,11 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) ->
> kernel::fmt::Result { 31:16 frts_err_code as u16;
> });
>
> +register!(NV_PBUS_BAR0_WINDOW @ 0x00001700, "BAR0 window control for
> PRAMIN access" {
> + 25:24 target as u8, "Target memory (0=VRAM, 1=SYS_MEM_COH,
> 2=SYS_MEM_NONCOH)";
> + 23:0 window_base as u32, "Window base address (bits 39:16 of FB
> addr)"; +});
> +
> // PFB
>
> // The following two registers together hold the physical system memory
> address that is used by the
^ permalink raw reply
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Zhi Wang @ 2026-01-21 7:27 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <20260120204303.3229303-2-joelagnelf@nvidia.com>
On Tue, 20 Jan 2026 15:42:38 -0500
Joel Fernandes <joelagnelf@nvidia.com> wrote:
> Add a new module `clist` 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.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
snip
> +/// Initialize a `list_head` object to point to itself.
> +///
> +/// # Safety
> +///
> +/// `list` must be a valid pointer to a `list_head` object.
> +#[inline]
> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
> + // SAFETY: Caller guarantees `list` is a valid pointer to a
> `list_head`.
> + unsafe {
> + (*list).next = list;
> + (*list).prev = list;
> + }
> +}
> +
Might be better to have a C helper? since INIT_LIST_HEAD() has WRITE_ONCE()
for memory ordering. This one seems not equal to it.
Z.
> +/// Wraps a `list_head` object for use in intrusive linked lists.
> +///
> +/// # Invariants
> +///
> +/// - [`CListHead`] represents an allocated and valid `list_head`
> structure. +/// - Once a [`CListHead`] is created in Rust, it will not
> be modified by non-Rust code. +/// - All `list_head` for individual
> items are not modified for the lifetime of [`CListHead`].
> +#[repr(transparent)] +pub struct CListHead(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 allocated and initialized
> `list_head` structure.
> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> + #[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.
> + unsafe { &*ptr.cast() }
> + }
> +
> + /// Get the raw `list_head` pointer.
> + #[inline]
> + pub fn as_raw(&self) -> *mut bindings::list_head {
> + self.0.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 per type invariants.
> + // - The `next` pointer is guaranteed to be non-NULL.
> + unsafe { Self::from_raw((*raw).next) }
> + }
> +
> + /// Get the previous [`CListHead`] in the list.
> + #[inline]
> + pub fn prev(&self) -> &Self {
> + let raw = self.as_raw();
> + // SAFETY:
> + // - self.as_raw() is valid per type invariants.
> + // - The `prev` pointer is guaranteed to be non-NULL.
> + unsafe { Self::from_raw((*raw).prev) }
> + }
> +
> + /// 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 }
> + }
> +
> + /// Fallible pin-initializer that initializes and then calls user
> closure.
> + ///
> + /// Initializes the list head first, then passes `&CListHead` to
> the closure.
> + /// This hides the raw FFI pointer from the user.
> + pub fn try_init<E>(
> + init_func: impl FnOnce(&CListHead) -> Result<(), E>,
> + ) -> impl PinInit<Self, E> {
> + // SAFETY: init_list_head initializes the list_head to point to
> itself.
> + // After initialization, we create a reference to pass to the
> closure.
> + unsafe {
> + pin_init::pin_init_from_closure(move |slot: *mut Self| {
> + init_list_head(slot.cast());
> + // SAFETY: slot is now initialized, safe to create
> reference.
> + init_func(&*slot)
> + })
> + }
> + }
> +}
> +
> +// SAFETY: [`CListHead`] can be sent to any thread.
> +unsafe impl Send for CListHead {}
> +
> +// SAFETY: [`CListHead`] can be shared among threads as it is not
> modified +// by non-Rust code per type invariants.
> +unsafe impl Sync for CListHead {}
> +
> +impl PartialEq for CListHead {
> + fn eq(&self, other: &Self) -> bool {
> + self.as_raw() == other.as_raw()
> + }
> +}
> +
> +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` macro or similar). +///
> +/// # Invariants
> +///
> +/// [`CListHeadIter`] is iterating over an allocated, initialized and
> valid list. +struct CListHeadIter<'a> {
> + current_head: &'a CListHead,
> + list_head: &'a CListHead,
> +}
> +
> +impl<'a> Iterator for CListHeadIter<'a> {
> + type Item = &'a CListHead;
> +
> + #[inline]
> + fn next(&mut self) -> Option<Self::Item> {
> + // Advance to next node.
> + let next = self.current_head.next();
> +
> + // Check if we've circled back to the sentinel head.
> + if next == self.list_head {
> + None
> + } else {
> + self.current_head = next;
> + Some(self.current_head)
> + }
> + }
> +}
> +
> +impl<'a> FusedIterator for CListHeadIter<'a> {}
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head 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
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is
> the list's sentinel. +/// - `OFFSET` is the byte offset of the
> `list_head` field within the struct that `T` wraps. +/// - All the
> list's `list_head` nodes are allocated and have valid next/prev
> pointers. +/// - The underlying `list_head` (and entire list) is not
> modified for the lifetime `'a`. +pub struct CList<'a, T, const OFFSET:
> usize> {
> + head: &'a CListHead,
> + _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T, const OFFSET: usize> CList<'a, T, OFFSET> {
> + /// Create a typed [`CList`] from a raw sentinel `list_head`
> pointer.
> + ///
> + /// # Safety
> + ///
> + /// - `ptr` must be a valid pointer to an allocated and initialized
> `list_head` structure
> + /// representing a list sentinel.
> + /// - `ptr` must remain valid and unmodified 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(ptr: *mut bindings::list_head) -> Self {
> + Self {
> + // SAFETY: Caller guarantees `ptr` is a valid, sentinel
> `list_head` object.
> + head: unsafe { CListHead::from_raw(ptr) },
> + _phantom: PhantomData,
> + }
> + }
> +
> + /// Get the raw sentinel `list_head` pointer.
> + #[inline]
> + pub fn as_raw(&self) -> *mut bindings::list_head {
> + self.head.as_raw()
> + }
> +
> + /// Check if the list is empty.
> + #[inline]
> + pub fn is_empty(&self) -> bool {
> + let raw = self.as_raw();
> + // SAFETY: self.as_raw() is valid per type invariants.
> + unsafe { (*raw).next == raw }
> + }
> +
> + /// Create an iterator over typed items.
> + #[inline]
> + pub fn iter(&self) -> CListIter<'a, T, OFFSET> {
> + CListIter {
> + head_iter: CListHeadIter {
> + current_head: self.head,
> + list_head: self.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;
> +
> + fn next(&mut self) -> Option<Self::Item> {
> + let head = self.head_iter.next()?;
> +
> + // Convert to item using OFFSET.
> + // SAFETY: `item_ptr` calculation from `OFFSET` (calculated
> using offset_of!)
> + // is valid per 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` pointing to
> a list that remains +/// unmodified for the lifetime of the rust
> [`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. +/// - The macro is called from an
> unsafe block. +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [`crate::clist`] 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| ::core::ptr::addr_of!((*p).$($field).+);
> +
> + // Calculate offset and create `CList`.
> + const OFFSET: usize = ::core::mem::offset_of!($c_type,
> $($field).+);
> + $crate::clist::CList::<$rust_type, OFFSET>::from_raw($head)
> + }};
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index f812cf120042..cd7e6a1055b0 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -75,6 +75,7 @@
> pub mod bug;
> #[doc(hidden)]
> pub mod build_assert;
> +pub mod clist;
> pub mod clk;
> #[cfg(CONFIG_CONFIGFS_FS)]
> pub mod configfs;
^ permalink raw reply
* Re: [PATCH v7 2/2] staging: fbtft: Make framebuffer registration message debug-only
From: Chintan Patel @ 2026-01-21 2:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-fbdev, linux-staging, linux-omap, linux-kernel, dri-devel,
tzimmermann, andy, deller, gregkh
In-Reply-To: <aW3gCgB1YAsjuOZ7@smile.fi.intel.com>
On 1/18/26 23:40, Andy Shevchenko wrote:
> On Fri, Jan 16, 2026 at 08:29:31PM -0800, Chintan Patel wrote:
>> The framebuffer registration message is informational only and not
>> useful during normal operation. Convert it to debug-level logging to
>> keep the driver quiet when working correctly.
>
> Suggested-by: Greg ...?
Ahh.. thanks for pointing it out! Will send v8.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
^ permalink raw reply
* Re: [PATCH RFC v6 01/26] rust: clist: Add support to interface with C linked lists
From: Gary Guo @ 2026-01-20 23:48 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <20260120204303.3229303-2-joelagnelf@nvidia.com>
On Tue Jan 20, 2026 at 8:42 PM GMT, Joel Fernandes wrote:
> Add a new module `clist` 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.
This should read "CList".
---
I was quite dubious about the patch just from the title (everybody knows how
easy a linked list is in Rust), but it turns out it is not as concerning as I
expected, mostly due to the read-only nature of the particular implementation
(a lot of the safety comments would be much more difficult to justify, say, if
it's mutable). That said, still a lot of feedbacks below.
I think something like is okay in the short term. However, there's an growing
interest in getting our Rust list API improved, so it could be ideal if
eventually the Rust list can be capable of handling FFI lists, too.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> MAINTAINERS | 7 +
> rust/helpers/helpers.c | 1 +
> rust/helpers/list.c | 12 ++
> rust/kernel/clist.rs | 357 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 5 files changed, 378 insertions(+)
> create mode 100644 rust/helpers/list.c
> create mode 100644 rust/kernel/clist.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0d044a58cbfe..b76988c38045 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22936,6 +22936,13 @@ F: rust/kernel/init.rs
> F: rust/pin-init/
> K: \bpin-init\b|pin_init\b|PinInit
>
> +RUST TO C LIST INTERFACES
> +M: Joel Fernandes <joelagnelf@nvidia.com>
> +M: Alexandre Courbot <acourbot@nvidia.com>
> +L: rust-for-linux@vger.kernel.org
> +S: Maintained
> +F: rust/kernel/clist.rs
> +
> RXRPC SOCKETS (AF_RXRPC)
> M: David Howells <dhowells@redhat.com>
> M: Marc Dionne <marc.dionne@auristor.com>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c..634fa2386bbb 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -32,6 +32,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..6044979c7a2e
> --- /dev/null
> +++ b/rust/helpers/list.c
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Helpers for C Circular doubly linked list implementation.
> + */
> +
> +#include <linux/list.h>
> +
> +void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
> +{
> + list_add_tail(new, head);
> +}
> diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
> new file mode 100644
> index 000000000000..91754ae721b9
> --- /dev/null
> +++ b/rust/kernel/clist.rs
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A C doubly circular intrusive linked list interface for rust code.
> +//!
> +//! # Examples
> +//!
> +//! ```
> +//! use kernel::{
> +//! bindings,
> +//! clist::init_list_head,
> +//! 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(crate) 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 { 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: pointers are to allocated test objects with a list_head field.
> +//! # unsafe {
> +//! # (*ptr).value = i as i32 * 10;
> +//! # // addr_of_mut!() computes address of link directly as link is uninitialized.
> +//! # init_list_head(core::ptr::addr_of_mut!((*ptr).link));
> +//! # 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:
> +//! // struct SampleItemC {
> +//! // int value;
> +//! // struct list_head link;
> +//! // };
> +//! //
> +//! #[repr(transparent)]
> +//! pub(crate) struct Item(Opaque<SampleItemC>);
> +//!
> +//! impl Item {
> +//! pub(crate) 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, items are [`SampleItemC`] with embedded `link` field.
> +//! 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::PinInit;
> +
> +/// Initialize a `list_head` object to point to itself.
> +///
> +/// # Safety
> +///
> +/// `list` must be a valid pointer to a `list_head` object.
> +#[inline]
> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
> + // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
> + unsafe {
> + (*list).next = list;
> + (*list).prev = list;
This needs to be an atomic write or it'll depart from the C implementation.
> + }
> +}
I don't think we want to publicly expose this! I've not found a user in the
subsequent patch, too.
Alice suggested to move this to bindings in v3 which I think is a good idea.
Also, even though it's against Rust name convention, for bindings we should use
the exact name as C (so INIT_LIST_HEAD).
> +
> +/// Wraps a `list_head` object for use in intrusive linked lists.
> +///
> +/// # Invariants
> +///
> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
> +#[repr(transparent)]
> +pub struct CListHead(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 allocated and initialized `list_head` structure.
> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> + #[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.
> + unsafe { &*ptr.cast() }
> + }
> +
> + /// Get the raw `list_head` pointer.
> + #[inline]
> + pub fn as_raw(&self) -> *mut bindings::list_head {
> + self.0.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 per type invariants.
> + // - The `next` pointer is guaranteed to be non-NULL.
> + unsafe { Self::from_raw((*raw).next) }
> + }
> +
> + /// Get the previous [`CListHead`] in the list.
> + #[inline]
> + pub fn prev(&self) -> &Self {
> + let raw = self.as_raw();
> + // SAFETY:
> + // - self.as_raw() is valid per type invariants.
> + // - The `prev` pointer is guaranteed to be non-NULL.
> + unsafe { Self::from_raw((*raw).prev) }
> + }
> +
> + /// 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 }
While is this checking both prev and next? `list_empty` is just
`READ_ONCE(head->next) == head`.
> + }
> +
> + /// Fallible pin-initializer that initializes and then calls user closure.
> + ///
> + /// Initializes the list head first, then passes `&CListHead` to the closure.
> + /// This hides the raw FFI pointer from the user.
> + pub fn try_init<E>(
> + init_func: impl FnOnce(&CListHead) -> Result<(), E>,
> + ) -> impl PinInit<Self, E> {
> + // SAFETY: init_list_head initializes the list_head to point to itself.
> + // After initialization, we create a reference to pass to the closure.
> + unsafe {
> + pin_init::pin_init_from_closure(move |slot: *mut Self| {
> + init_list_head(slot.cast());
> + // SAFETY: slot is now initialized, safe to create reference.
> + init_func(&*slot)
Why is this callback necessary? The user can just create the list head and
then reference it later? I don't see what this specifically gains over just
doing
fn new() -> impl PinInit<Self>;
and have user-side
list <- CListHead::new(),
_: {
do_want_ever(&list)
}
> + })
> + }
> + }
> +}
> +
> +// SAFETY: [`CListHead`] can be sent to any thread.
> +unsafe impl Send for CListHead {}
> +
> +// SAFETY: [`CListHead`] can be shared among threads as it is not modified
> +// by non-Rust code per type invariants.
> +unsafe impl Sync for CListHead {}
> +
> +impl PartialEq for CListHead {
> + fn eq(&self, other: &Self) -> bool {
> + self.as_raw() == other.as_raw()
Or just `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` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// [`CListHeadIter`] is iterating over an allocated, initialized and valid list.
> +struct CListHeadIter<'a> {
> + current_head: &'a CListHead,
> + list_head: &'a CListHead,
> +}
> +
> +impl<'a> Iterator for CListHeadIter<'a> {
> + type Item = &'a CListHead;
> +
> + #[inline]
> + fn next(&mut self) -> Option<Self::Item> {
> + // Advance to next node.
> + let next = self.current_head.next();
> +
> + // Check if we've circled back to the sentinel head.
> + if next == self.list_head {
> + None
> + } else {
> + self.current_head = next;
> + Some(self.current_head)
> + }
I think this could match the C iterator behaviour. When the iterator is created,
a `next` is done first, and then subsequently you only need to check if
`current_head` is `list_head`.
This is slightly better because the condition check does not need to dereference
a pointer.
> + }
> +}
> +
> +impl<'a> FusedIterator for CListHeadIter<'a> {}
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head 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
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
> +/// - `OFFSET` is the byte offset of the `list_head` field within the struct that `T` wraps.
> +/// - All the list's `list_head` nodes are allocated and have valid next/prev pointers.
> +/// - The underlying `list_head` (and entire list) is not modified for the lifetime `'a`.
> +pub struct CList<'a, T, const OFFSET: usize> {
> + head: &'a CListHead,
> + _phantom: PhantomData<&'a T>,
> +}
Is there a reason that this is not
#[repr(transparent)]
struct CList(CListHead)
? We typically want to avoid putting reference inside the struct if it can be on
the outside. This allows `&self` to be a single level of reference, not too.
It also means that you can just write `&CList<_>` in many cases, and doesn't need
`CList<'_, T>` (plus all the benefits of a reference).
> +
> +impl<'a, T, const OFFSET: usize> CList<'a, T, OFFSET> {
> + /// Create a typed [`CList`] from a raw sentinel `list_head` pointer.
> + ///
> + /// # Safety
> + ///
> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
> + /// representing a list sentinel.
> + /// - `ptr` must remain valid and unmodified 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(ptr: *mut bindings::list_head) -> Self {
> + Self {
> + // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
> + head: unsafe { CListHead::from_raw(ptr) },
> + _phantom: PhantomData,
> + }
> + }
> +
> + /// Get the raw sentinel `list_head` pointer.
> + #[inline]
> + pub fn as_raw(&self) -> *mut bindings::list_head {
> + self.head.as_raw()
> + }
> +
> + /// Check if the list is empty.
> + #[inline]
> + pub fn is_empty(&self) -> bool {
> + let raw = self.as_raw();
> + // SAFETY: self.as_raw() is valid per type invariants.
> + unsafe { (*raw).next == raw }
`self.head.is_linked()`?
> + }
> +
> + /// Create an iterator over typed items.
> + #[inline]
> + pub fn iter(&self) -> CListIter<'a, T, OFFSET> {
> + CListIter {
> + head_iter: CListHeadIter {
> + current_head: self.head,
> + list_head: self.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;
> +
> + fn next(&mut self) -> Option<Self::Item> {
> + let head = self.head_iter.next()?;
> +
> + // Convert to item using OFFSET.
> + // SAFETY: `item_ptr` calculation from `OFFSET` (calculated using offset_of!)
> + // is valid per 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` pointing to a list that remains
> +/// unmodified for the lifetime of the rust [`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.
> +/// - The macro is called from an unsafe block.
This is not a safe requirement, probably lift it up and say "This is an unsafe
macro.".
> +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [`crate::clist`] 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| ::core::ptr::addr_of!((*p).$($field).+);
`&raw const` is preferred now.
> +
> + // Calculate offset and create `CList`.
> + const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
> + $crate::clist::CList::<$rust_type, OFFSET>::from_raw($head)
> + }};
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index f812cf120042..cd7e6a1055b0 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -75,6 +75,7 @@
> pub mod bug;
> #[doc(hidden)]
> pub mod build_assert;
> +pub mod clist;
Can we keep this pub(crate)?
Best,
Gary
> pub mod clk;
> #[cfg(CONFIG_CONFIGFS_FS)]
> pub mod configfs;
^ permalink raw reply
* [PATCH RFC v6 26/26] nova-core: mm: Add BarUser to struct Gpu and create at boot
From: Joel Fernandes @ 2026-01-20 20:43 UTC (permalink / raw)
To: linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev, Joel Fernandes
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
Add a BarUser field to struct Gpu and eagerly create it during GPU
initialization. The BarUser provides the BAR1 user interface for CPU
access to GPU virtual memory through the GPU's MMU.
The BarUser is initialized using BAR1 PDE base address from GSP static
info, MMU version and BAR1 size obtained from platform device.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index dd05ad23f763..15d8d42ecfa8 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -26,7 +26,12 @@
commands::GetGspStaticInfoReply,
Gsp, //
},
- mm::GpuMm,
+ mm::{
+ bar_user::BarUser,
+ pagetable::MmuVersion,
+ GpuMm,
+ VramAddress, //
+ },
regs,
};
@@ -35,6 +40,7 @@
struct BootParams {
usable_vram_start: u64,
usable_vram_size: u64,
+ bar1_pde_base: u64,
}
macro_rules! define_chipset {
@@ -271,6 +277,8 @@ pub(crate) struct Gpu {
gsp: Gsp,
/// Static GPU information from GSP.
gsp_static_info: GetGspStaticInfoReply,
+ /// BAR1 user interface for CPU access to GPU virtual memory.
+ bar_user: BarUser,
}
impl Gpu {
@@ -284,6 +292,7 @@ pub(crate) fn new<'a>(
let boot_params: Cell<BootParams> = Cell::new(BootParams {
usable_vram_start: 0,
usable_vram_size: 0,
+ bar1_pde_base: 0,
});
try_pin_init!(Self {
@@ -328,6 +337,7 @@ pub(crate) fn new<'a>(
boot_params.set(BootParams {
usable_vram_start: usable_vram.start,
usable_vram_size: usable_vram.end - usable_vram.start,
+ bar1_pde_base: info.bar1_pde_base(),
});
info
@@ -344,6 +354,16 @@ pub(crate) fn new<'a>(
})?
},
+ // Create BAR1 user interface for CPU access to GPU virtual memory.
+ // Uses the BAR1 PDE base from GSP and full BAR1 size for VA space.
+ bar_user: {
+ let params = boot_params.get();
+ let pdb_addr = VramAddress::new(params.bar1_pde_base);
+ let mmu_version = MmuVersion::from(spec.chipset.arch());
+ let bar1_size = pdev.resource_len(1)?;
+ BarUser::new(pdb_addr, mmu_version, bar1_size)?
+ },
+
bar: devres_bar,
})
}
--
2.34.1
^ permalink raw reply related
* [PATCH RFC v6 25/26] nova-core: mm: Use usable VRAM region for buddy allocator
From: Joel Fernandes @ 2026-01-20 20:43 UTC (permalink / raw)
To: linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev, Joel Fernandes
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
The buddy allocator manages the actual usable VRAM. On my GA102 Ampere
with 24GB video memory, that is ~23.7GB on a 24GB GPU enabling proper
GPU memory allocation for driver use.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 62 ++++++++++++++++++++++-----
drivers/gpu/nova-core/gsp/boot.rs | 7 ++-
drivers/gpu/nova-core/gsp/commands.rs | 2 -
3 files changed, 57 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index a1bcf6679e2a..dd05ad23f763 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+use core::cell::Cell;
+
use kernel::{
device,
devres::Devres,
@@ -7,7 +9,7 @@
gpu::buddy::GpuBuddyParams,
pci,
prelude::*,
- sizes::{SZ_1M, SZ_4K},
+ sizes::SZ_4K,
sync::Arc, //
};
@@ -28,6 +30,13 @@
regs,
};
+/// Parameters extracted from GSP boot for initializing memory subsystems.
+#[derive(Clone, Copy)]
+struct BootParams {
+ usable_vram_start: u64,
+ usable_vram_size: u64,
+}
+
macro_rules! define_chipset {
({ $($variant:ident = $value:expr),* $(,)* }) =>
{
@@ -270,6 +279,13 @@ pub(crate) fn new<'a>(
devres_bar: Arc<Devres<Bar0>>,
bar: &'a Bar0,
) -> impl PinInit<Self, Error> + 'a {
+ // Cell to share boot parameters between GSP boot and subsequent initializations.
+ // Contains usable VRAM region from FbLayout and BAR1 PDE base from GSP info.
+ let boot_params: Cell<BootParams> = Cell::new(BootParams {
+ usable_vram_start: 0,
+ usable_vram_size: 0,
+ });
+
try_pin_init!(Self {
spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
dev_info!(pdev.as_ref(),"NVIDIA ({})\n", spec);
@@ -291,18 +307,42 @@ pub(crate) fn new<'a>(
sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?,
- // Create GPU memory manager owning memory management resources.
- // This will be initialized with the usable VRAM region from GSP in a later
- // patch. For now, we use a placeholder of 1MB.
- mm: GpuMm::new(devres_bar.clone(), GpuBuddyParams {
- base_offset_bytes: 0,
- physical_memory_size_bytes: SZ_1M as u64,
- chunk_size_bytes: SZ_4K as u64,
- })?,
-
gsp <- Gsp::new(pdev),
- gsp_static_info: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)?.0 },
+ // Boot GSP and extract usable VRAM region for buddy allocator.
+ gsp_static_info: {
+ let (info, fb_layout) = gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)?;
+
+ let usable_vram = fb_layout.usable_vram.as_ref().ok_or_else(|| {
+ dev_err!(pdev.as_ref(), "No usable FB regions found from GSP\n");
+ ENODEV
+ })?;
+
+ dev_info!(
+ pdev.as_ref(),
+ "Using FB region: {:#x}..{:#x}\n",
+ usable_vram.start,
+ usable_vram.end
+ );
+
+ boot_params.set(BootParams {
+ usable_vram_start: usable_vram.start,
+ usable_vram_size: usable_vram.end - usable_vram.start,
+ });
+
+ info
+ },
+
+ // Create GPU memory manager owning memory management resources.
+ // Uses the usable VRAM region from GSP for buddy allocator.
+ mm: {
+ let params = boot_params.get();
+ GpuMm::new(devres_bar.clone(), GpuBuddyParams {
+ base_offset_bytes: params.usable_vram_start,
+ physical_memory_size_bytes: params.usable_vram_size,
+ chunk_size_bytes: SZ_4K as u64,
+ })?
+ },
bar: devres_bar,
})
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 75f949bc4864..a034e2e80a4b 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -150,7 +150,7 @@ pub(crate) fn boot(
let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset, FIRMWARE_VERSION), GFP_KERNEL)?;
- let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
+ let mut fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
dev_dbg!(dev, "{:#x?}\n", fb_layout);
Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
@@ -252,6 +252,11 @@ pub(crate) fn boot(
Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable: {:?}\n", e),
}
+ // Populate usable VRAM from GSP response.
+ if let Some((base, size)) = info.usable_fb_region() {
+ fb_layout.set_usable_vram(base, size);
+ }
+
Ok((info, fb_layout))
}
}
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index d619cf294b9c..4a7eda512789 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -191,7 +191,6 @@ pub(crate) struct GetGspStaticInfoReply {
gpu_name: [u8; 64],
bar1_pde_base: u64,
/// First usable FB region (base, size) for memory allocation.
- #[expect(dead_code)]
usable_fb_region: Option<(u64, u64)>,
}
@@ -242,7 +241,6 @@ pub(crate) fn bar1_pde_base(&self) -> u64 {
/// Returns the usable FB region (base, size) for driver allocation which is
/// already retrieved from the GSP.
- #[expect(dead_code)]
pub(crate) fn usable_fb_region(&self) -> Option<(u64, u64)> {
self.usable_fb_region
}
--
2.34.1
^ permalink raw reply related
* [PATCH RFC v6 24/26] nova-core: fb: Add usable_vram field to FbLayout
From: Joel Fernandes @ 2026-01-20 20:43 UTC (permalink / raw)
To: linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev, Joel Fernandes
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
Add usable_vram field to FbLayout to store the usable VRAM region for
driver allocations. This is populated after GSP boot with the region
extracted from GSP's fbRegionInfoParams.
FbLayout is now a two-phase structure:
1. new() computes firmware layout from hardware
2. set_usable_vram() populates usable region from GSP
The new usable_vram field represents the actual usable VRAM region
(~23.7GB on a 24GB GPU GA102 Ampere GPU).
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/fb.rs | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index c62abcaed547..779447952b19 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -97,6 +97,10 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
/// Layout of the GPU framebuffer memory.
///
/// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
+///
+/// This structure is populated in 2 steps:
+/// 1. [`FbLayout::new()`] computes firmware layout from hardware.
+/// 2. [`FbLayout::set_usable_vram()`] populates usable region from GSP response.
#[derive(Debug)]
pub(crate) struct FbLayout {
/// Range of the framebuffer. Starts at `0`.
@@ -111,10 +115,14 @@ pub(crate) struct FbLayout {
pub(crate) elf: Range<u64>,
/// WPR2 heap.
pub(crate) wpr2_heap: Range<u64>,
- /// WPR2 region range, starting with an instance of `GspFwWprMeta`.
+ /// WPR2 region range, starting with an instance of [`GspFwWprMeta`].
pub(crate) wpr2: Range<u64>,
+ /// Non-WPR heap carved before WPR2, used by GSP firmware.
pub(crate) heap: Range<u64>,
pub(crate) vf_partition_count: u8,
+ /// Usable VRAM region for driver allocations (from GSP `fbRegionInfoParams`).
+ /// Initially [`None`], populated after GSP boot with usable region info.
+ pub(crate) usable_vram: Option<Range<u64>>,
}
impl FbLayout {
@@ -212,6 +220,19 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
wpr2,
heap,
vf_partition_count: 0,
+ usable_vram: None,
})
}
+
+ /// Set the usable VRAM region from GSP response.
+ ///
+ /// Called after GSP boot with the first usable region extracted from
+ /// GSP's `fbRegionInfoParams`. Usable regions are those that:
+ /// - Are not reserved for firmware internal use.
+ /// - Are not protected (hardware-enforced access restrictions).
+ /// - Support compression (can use GPU memory compression for bandwidth).
+ /// - Support ISO (isochronous memory for display requiring guaranteed bandwidth).
+ pub(crate) fn set_usable_vram(&mut self, base: u64, size: u64) {
+ self.usable_vram = Some(base..base.saturating_add(size));
+ }
}
--
2.34.1
^ permalink raw reply related
* [PATCH RFC v6 23/26] nova-core: gsp: Extract usable FB region from GSP
From: Joel Fernandes @ 2026-01-20 20:43 UTC (permalink / raw)
To: linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev, Joel Fernandes
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
usable FB region from GSP's fbRegionInfoParams. Usable regions are those
that are not reserved or protected.
The extracted region is stored in GetGspStaticInfoReply and exposed via
usable_fb_region() API for use by the memory subsystem.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/gsp/commands.rs | 13 +++++++++-
drivers/gpu/nova-core/gsp/fw/commands.rs | 30 ++++++++++++++++++++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 311f65f8367b..d619cf294b9c 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -186,10 +186,13 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
}
}
-/// The reply from the GSP to the [`GetGspInfo`] command.
+/// The reply from the GSP to the [`GetGspStaticInfo`] command.
pub(crate) struct GetGspStaticInfoReply {
gpu_name: [u8; 64],
bar1_pde_base: u64,
+ /// First usable FB region (base, size) for memory allocation.
+ #[expect(dead_code)]
+ usable_fb_region: Option<(u64, u64)>,
}
impl MessageFromGsp for GetGspStaticInfoReply {
@@ -204,6 +207,7 @@ fn read(
Ok(GetGspStaticInfoReply {
gpu_name: msg.gpu_name_str(),
bar1_pde_base: msg.bar1_pde_base(),
+ usable_fb_region: msg.first_usable_fb_region(),
})
}
}
@@ -235,6 +239,13 @@ pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
pub(crate) fn bar1_pde_base(&self) -> u64 {
self.bar1_pde_base
}
+
+ /// Returns the usable FB region (base, size) for driver allocation which is
+ /// already retrieved from the GSP.
+ #[expect(dead_code)]
+ pub(crate) fn usable_fb_region(&self) -> Option<(u64, u64)> {
+ self.usable_fb_region
+ }
}
/// Send the [`GetGspInfo`] command and awaits for its reply.
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index f069f4092911..cc1cf4bd52ea 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -122,6 +122,36 @@ impl GspStaticConfigInfo {
pub(crate) fn bar1_pde_base(&self) -> u64 {
self.0.bar1PdeBase
}
+
+ /// Extract the first usable FB region from GSP firmware data.
+ ///
+ /// Returns the first region suitable for driver memory allocation as a base,size tuple.
+ /// Usable regions are those that:
+ /// - Are not reserved for firmware internal use.
+ /// - Are not protected (hardware-enforced access restrictions).
+ /// - Support compression (can use GPU memory compression for bandwidth).
+ /// - Support ISO (isochronous memory for display requiring guaranteed bandwidth).
+ pub(crate) fn first_usable_fb_region(&self) -> Option<(u64, u64)> {
+ let fb_info = &self.0.fbRegionInfoParams;
+ for i in 0..fb_info.numFBRegions as usize {
+ if let Some(reg) = fb_info.fbRegion.get(i) {
+ // Skip malformed regions where limit < base.
+ if reg.limit < reg.base {
+ continue;
+ }
+ // Filter: not reserved, not protected, supports compression and ISO.
+ if reg.reserved == 0
+ && reg.bProtected == 0
+ && reg.supportCompressed != 0
+ && reg.supportISO != 0
+ {
+ let size = reg.limit - reg.base + 1;
+ return Some((reg.base, size));
+ }
+ }
+ }
+ None
+ }
}
// SAFETY: Padding is explicit and will not contain uninitialized data.
--
2.34.1
^ permalink raw reply related
* [PATCH RFC v6 22/26] nova-core: mm: Add PRAMIN aperture self-tests
From: Joel Fernandes @ 2026-01-20 20:42 UTC (permalink / raw)
To: linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev, Joel Fernandes
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
Add self-tests for the PRAMIN aperture mechanism to verify correct
operation during GPU probe. The tests validate various alignment
requirements and corner cases.
The tests are default disabled and behind CONFIG_NOVA_PRAMIN_SELFTESTS
When enabled, tests run after GSP boot during probe.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/Kconfig | 11 ++
drivers/gpu/nova-core/gpu.rs | 14 +++
drivers/gpu/nova-core/mm/pramin.rs | 160 +++++++++++++++++++++++++++++
3 files changed, 185 insertions(+)
diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
index 257bca5aa0ef..cbdbc1fb02b2 100644
--- a/drivers/gpu/nova-core/Kconfig
+++ b/drivers/gpu/nova-core/Kconfig
@@ -25,3 +25,14 @@ config NOVA_MM_SELFTESTS
BAR1 virtual memory mapping functionality.
This is a testing option and is default-disabled.
+
+config NOVA_PRAMIN_SELFTESTS
+ bool "PRAMIN self-tests"
+ depends on NOVA_CORE
+ default n
+ help
+ Enable self-tests for the PRAMIN aperture mechanism. When enabled,
+ basic tests are run during GPU probe after GSP boot to
+ verify PRAMIN functionality.
+
+ This is a testing option and is default-disabled.
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 938828508f2c..a1bcf6679e2a 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -324,10 +324,24 @@ pub(crate) fn run_selftests(
mut self: Pin<&mut Self>,
pdev: &pci::Device<device::Bound>,
) -> Result {
+ self.as_mut().run_pramin_selftest(pdev)?;
self.as_mut().run_mm_selftest(pdev)?;
Ok(())
}
+ fn run_pramin_selftest(self: Pin<&mut Self>, pdev: &pci::Device<device::Bound>) -> Result {
+ #[cfg(CONFIG_NOVA_PRAMIN_SELFTESTS)]
+ {
+ use crate::mm::pagetable::MmuVersion;
+
+ let mmu_version = MmuVersion::from(self.spec.chipset.arch());
+ crate::mm::pramin::run_self_test(pdev.as_ref(), self.bar.clone(), mmu_version)?;
+ }
+
+ let _ = pdev; // Suppress unused warning when selftests disabled.
+ Ok(())
+ }
+
fn run_mm_selftest(mut self: Pin<&mut Self>, pdev: &pci::Device<device::Bound>) -> Result {
#[cfg(CONFIG_NOVA_MM_SELFTESTS)]
{
diff --git a/drivers/gpu/nova-core/mm/pramin.rs b/drivers/gpu/nova-core/mm/pramin.rs
index 6a7ea2dc7d77..06384fb24841 100644
--- a/drivers/gpu/nova-core/mm/pramin.rs
+++ b/drivers/gpu/nova-core/mm/pramin.rs
@@ -242,3 +242,163 @@ unsafe impl Send for Window {}
// SAFETY: `Window` requires `&mut self` for all accessors.
unsafe impl Sync for Window {}
+
+/// Run PRAMIN self-tests during boot if self-tests are enabled.
+#[cfg(CONFIG_NOVA_PRAMIN_SELFTESTS)]
+pub(crate) fn run_self_test(
+ dev: &kernel::device::Device,
+ bar: Arc<Devres<Bar0>>,
+ mmu_version: super::pagetable::MmuVersion,
+) -> Result {
+ use super::pagetable::MmuVersion;
+
+ // PRAMIN support is only for MMU v2 for now (Turing/Ampere/Ada).
+ if mmu_version != MmuVersion::V2 {
+ dev_info!(
+ dev,
+ "PRAMIN: Skipping self-tests for MMU {:?} (only V2 supported)\n",
+ mmu_version
+ );
+ return Ok(());
+ }
+
+ dev_info!(dev, "PRAMIN: Starting self-test...\n");
+
+ let mut win = Window::new(bar)?;
+
+ // Use offset 0x1000 as test area.
+ let base: usize = 0x1000;
+
+ // Test 1: Read/write at byte-aligned locations.
+ for i in 0u8..4 {
+ let offset = base + 1 + usize::from(i); // Offsets 0x1001, 0x1002, 0x1003, 0x1004
+ let val = 0xA0 + i;
+ win.try_write8(offset, val)?;
+ let read_val = win.try_read8(offset)?;
+ if read_val != val {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - offset {:#x}: wrote {:#x}, read {:#x}\n",
+ offset,
+ val,
+ read_val
+ );
+ return Err(EIO);
+ }
+ }
+
+ // Test 2: Write `u32` and read back as `u8`s.
+ let test2_offset = base + 0x10;
+ let test2_val: u32 = 0xDEADBEEF;
+ win.try_write32(test2_offset, test2_val)?;
+
+ // Read back as individual bytes (little-endian: EF BE AD DE).
+ let expected_bytes: [u8; 4] = [0xEF, 0xBE, 0xAD, 0xDE];
+ for (i, &expected) in expected_bytes.iter().enumerate() {
+ let read_val = win.try_read8(test2_offset + i)?;
+ if read_val != expected {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - offset {:#x}: expected {:#x}, read {:#x}\n",
+ test2_offset + i,
+ expected,
+ read_val
+ );
+ return Err(EIO);
+ }
+ }
+
+ // Test 3: Window repositioning across 1MB boundaries.
+ // Write to offset > 1MB to trigger window slide, then verify.
+ let test3_offset_a: usize = base; // First 1MB region.
+ let test3_offset_b: usize = 0x200000 + base; // 2MB + base (different 1MB region).
+ let val_a: u32 = 0x11111111;
+ let val_b: u32 = 0x22222222;
+
+ // Write to first region.
+ win.try_write32(test3_offset_a, val_a)?;
+
+ // Write to second region (triggers window reposition).
+ win.try_write32(test3_offset_b, val_b)?;
+
+ // Read back from second region.
+ let read_b = win.try_read32(test3_offset_b)?;
+ if read_b != val_b {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - offset {:#x}: expected {:#x}, read {:#x}\n",
+ test3_offset_b,
+ val_b,
+ read_b
+ );
+ return Err(EIO);
+ }
+
+ // Read back from first region (triggers window reposition again).
+ let read_a = win.try_read32(test3_offset_a)?;
+ if read_a != val_a {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - offset {:#x}: expected {:#x}, read {:#x}\n",
+ test3_offset_a,
+ val_a,
+ read_a
+ );
+ return Err(EIO);
+ }
+
+ // Test 4: Invalid offset rejection (beyond 40-bit address space).
+ {
+ // 40-bit address space limit check.
+ let invalid_offset: usize = MAX_VRAM_OFFSET + 1;
+ let result = win.try_read32(invalid_offset);
+ if result.is_ok() {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - read at invalid offset {:#x} should have failed\n",
+ invalid_offset
+ );
+ return Err(EIO);
+ }
+ }
+
+ // Test 5: Misaligned multi-byte access rejection.
+ // Verify that misaligned `u16`/`u32`/`u64` accesses are properly rejected.
+ {
+ // `u16` at odd offset (not 2-byte aligned).
+ let offset_u16 = base + 0x21;
+ if win.try_write16(offset_u16, 0xABCD).is_ok() {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - misaligned u16 write at {:#x} should have failed\n",
+ offset_u16
+ );
+ return Err(EIO);
+ }
+
+ // `u32` at 2-byte-aligned (not 4-byte-aligned) offset.
+ let offset_u32 = base + 0x32;
+ if win.try_write32(offset_u32, 0x12345678).is_ok() {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - misaligned u32 write at {:#x} should have failed\n",
+ offset_u32
+ );
+ return Err(EIO);
+ }
+
+ // `u64` read at 4-byte-aligned (not 8-byte-aligned) offset.
+ let offset_u64 = base + 0x44;
+ if win.try_read64(offset_u64).is_ok() {
+ dev_err!(
+ dev,
+ "PRAMIN: FAIL - misaligned u64 read at {:#x} should have failed\n",
+ offset_u64
+ );
+ return Err(EIO);
+ }
+ }
+
+ dev_info!(dev, "PRAMIN: All self-tests PASSED\n");
+ Ok(())
+}
--
2.34.1
^ permalink raw reply related
* [PATCH RFC v6 18/26] nova-core: mm: Add virtual address range tracking to VMM
From: Joel Fernandes @ 2026-01-20 20:42 UTC (permalink / raw)
To: linux-kernel
Cc: 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Alexey Ivanov, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev, Joel Fernandes
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
Extend the Virtual Memory Manager with optional virtual address range
tracking using a buddy allocator. This enables BarUser to allocate
contiguous virtual ranges for BAR1 mappings.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/mm/vmm.rs | 49 +++++++++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/mm/vmm.rs b/drivers/gpu/nova-core/mm/vmm.rs
index a5b4af9053a0..0ab80b84e55a 100644
--- a/drivers/gpu/nova-core/mm/vmm.rs
+++ b/drivers/gpu/nova-core/mm/vmm.rs
@@ -32,7 +32,9 @@
gpu::buddy::{
AllocatedBlocks,
BuddyFlags,
- GpuBuddyAllocParams, //
+ GpuBuddy,
+ GpuBuddyAllocParams,
+ GpuBuddyParams, //
},
prelude::*,
sizes::SZ_4K,
@@ -60,29 +62,48 @@
/// Virtual Memory Manager for a GPU address space.
///
/// Each [`Vmm`] instance manages a single address space identified by its Page
-/// Directory Base (`PDB`) address. The [`Vmm`] is used for BAR1 and BAR2 mappings.
+/// Directory Base (`PDB`) address. The [`Vmm`] is used for Channel, BAR1 and BAR2 mappings.
///
/// The [`Vmm`] tracks all page table allocations made during mapping operations
/// to ensure they remain valid for the lifetime of the address space.
+///
+/// It tracks virtual address allocations via a buddy allocator.
pub(crate) struct Vmm {
pdb_addr: VramAddress,
mmu_version: MmuVersion,
/// Page table allocations that must persist for the lifetime of mappings.
page_table_allocs: KVec<Arc<AllocatedBlocks>>,
+ /// Buddy allocator for virtual address range tracking.
+ virt_buddy: GpuBuddy,
}
impl Vmm {
/// Create a new [`Vmm`] for the given Page Directory Base address.
- pub(crate) fn new(pdb_addr: VramAddress, mmu_version: MmuVersion) -> Result<Self> {
+ ///
+ /// The [`Vmm`] will manage a virtual address space of `va_size` bytes using
+ /// a buddy allocator. This enables [`Vmm::alloc_vfn_range()`] for allocating
+ /// contiguous virtual ranges.
+ pub(crate) fn new(
+ pdb_addr: VramAddress,
+ mmu_version: MmuVersion,
+ va_size: u64,
+ ) -> Result<Self> {
// Only MMU v2 is supported for now.
if mmu_version != MmuVersion::V2 {
return Err(ENOTSUPP);
}
+ let virt_buddy = GpuBuddy::new(GpuBuddyParams {
+ base_offset_bytes: 0,
+ physical_memory_size_bytes: va_size,
+ chunk_size_bytes: SZ_4K as u64,
+ })?;
+
Ok(Self {
pdb_addr,
mmu_version,
page_table_allocs: KVec::new(),
+ virt_buddy,
})
}
@@ -96,6 +117,28 @@ pub(crate) fn mmu_version(&self) -> MmuVersion {
self.mmu_version
}
+ /// Allocate a contiguous virtual frame number range.
+ ///
+ /// Returns an [`Arc<AllocatedBlocks>`] representing the allocated range.
+ /// The allocation is automatically freed when the [`Arc`] is dropped.
+ pub(crate) fn alloc_vfn_range(&self, num_pages: usize) -> Result<(Vfn, Arc<AllocatedBlocks>)> {
+ let params = GpuBuddyAllocParams {
+ start_range_address: 0,
+ end_range_address: 0,
+ size_bytes: num_pages.checked_mul(PAGE_SIZE).ok_or(EOVERFLOW)? as u64,
+ min_block_size_bytes: SZ_4K as u64,
+ buddy_flags: BuddyFlags::try_new(BuddyFlags::CONTIGUOUS_ALLOCATION)?,
+ };
+
+ let alloc = self.virt_buddy.alloc_blocks(params)?;
+
+ // Get the starting offset from the first (and only, due to CONTIGUOUS) block.
+ let offset = alloc.iter().next().ok_or(ENOMEM)?.offset();
+ let vfn = Vfn::new(offset / PAGE_SIZE as u64);
+
+ Ok((vfn, alloc))
+ }
+
/// Allocate a new page table, zero it, and track the allocation.
///
/// This method ensures page table allocations persist for the lifetime of
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox