Message-ID: <3cc835b1-d9e6-4d91-a398-7ea9c8f4332a@nvidia.com>
Date: Wed, 22 Oct 2025 17:30:02 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 dakr@...nel.org
Cc: Alistair Popple <apopple@...dia.com>, Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>, bjorn3_gh@...tonmail.com,
 Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 John Hubbard <jhubbard@...dia.com>, Timur Tabi <ttabi@...dia.com>,
 joel@...lfernandes.org, Elle Rhumsaa <elle@...thered-steel.dev>,
 Daniel Almeida <daniel.almeida@...labora.com>, nouveau@...ts.freedesktop.org
Subject: Re: [PATCH 0/7] Pre-requisite patches for mm and irq in nova-core

Hi Alex,

On 10/22/2025 2:57 AM, Alexandre Courbot wrote:
> On Tue Oct 21, 2025 at 3:55 AM JST, Joel Fernandes wrote:
>> These patches have some prerequistes needed for nova-core as support is added
>> for memory management and interrupt handling. I rebased them on drm-rust-next
>> and would like them to be considered for the next merge window. I also included
>> a simple rust documentation patch fixing some issues I noticed while reading it :).
>>
>> The series adds support for the PRAMIN aperture mechanism, which is a
>> prerequisite for virtual memory as it is required to boot strap virtual memory
>> (we cannot write to VRAM using virtual memory because we need to write page
>> tables to VRAM in the first place).
>>
>> I also add page table related structures (mm/types.rs) using the bitfield
>> macro, which will be used for page table walking, memory mapping, etc. This is
>> currently unused code, because without physical memory allocation (using the
>> buddy allocator), we cannot use this code as page table pages need to be
>> allocated in the first place. However, I have included several examples in the
>> file about how these structures will be used. I have also simplified the code
>> keeping future additions to it for later.
>>
>> For interrupts, I only have added additional support for GSP's message queue
>> interrupt. I am working on adding support to the interrupt controller module
>> (VFN) which is the next thing for me to post after this series. I have it
>> prototyped and working, however I am currently making several changes to it
>> related to virtual functions. For now in this series, I just want to get the
>> GSP-specific patch out of the way, hence I am including it here.
>>
>> I also have added a patch for bitfield macro which constructs a bitfield struct
>> given its storage type. This is used in a later GSP interrupt patch in the
>> series to read from one register and write to another.
> 
> So IIUC, this series contains the following:
> 
> 1. Add PRAMIN support,
> 2. Add Page mapping support, albeit this is unexercised until we have a
>    user (e.g. buddy allocator),
> 3. Add Falcon interrupt support,
> 4. Add missing bitfield functionality, albeit not used yet,
> 5. Various documentation patches.
> 
> This is a bit confusing, as there is close to no logical relationship or
> dependency between these patches. I see potential for several different
> submissions here:
> 
> - The core documentation fix, as Miguel pointed out, since it should be
>   merged into the rust tree and not nova-core.
> - The bitfield patch is a useful addition and should be sent separately.
> - PRAMIN/Page mapping should come with code that exercices them, so
>   think they belong as the first patches of a series that ends with
>   basic memory allocation capabilities. But feel free to send a RFC if
>   you want early feedback.

Ah, so if we go with that - the mm patches here (Pramin and page structures
types) will then have to wait till we have DRM buddy bindings or a DRM buddy
written in rust (the latter of which I already have). That would be unfortunate
because it will take more time up in the future to rebase. But RFC is a good
idea too for just getting an advance review. I think we need to carefully choose
RFC versus merging small (even if unused) patches (more below).

> - The falcon interrupts patch does not seem to be used by the last two
>   patches? I guess it belongs to the series that will add support for
>   the interrupt controller.
No, it is independent. Yes this leads up to the interrupt handing feature, but
just to emphasize a bit, the motivation was to "get small patches" in so that we
don't need to do obvious things later (example, the VFN interrupt module is much
more complex than this GSP patch yet both are needed for interrupt handling, so
the GSP patch is a good candidate IMO for upstreaming in the next merge window).
Having small patches merged reduces future burden on both reviewers and the
developers. This is also not something new, for instance we don't have any users
of the PCI MSI IRQ allocation bindings in rust/, yet we merged those. I think
that is reasonable. RFC should be used too when it makes sense, but I think we
should also look into merging things in chunks to avoid future review/rebase
burden. There isn't one rule that fits all is my point, right? I mean just look
at the attempted bitfield move too, Nova is the only user yet we will move it
out. But one may ask why move it out until there are other users? It has to be
on a case-by-case basis..

> - Other documentation patches belong to the series that introduces the
>   feature they document.
Agreed with the splitting suggestions, I will split this series moving forward.
But I may still push for merging small patches where it makes sense, if that's
Ok with everyone.

(I do feel the PRAMIN and GSP patch both do fall under the 'small patch' category)

thanks,

 - Joel

Powered by blists - more mailing lists