Hi,

I'm sending this just a few days after v1, which is soon, but Timur Tabi
and Zhi Wang reviewed the whole series in that time, and that led to
quite a lot of fixes and lines changed. So I think it is helpful to
provide a v2 now, so that future reviewers can avoid rediscovering all
those issues.

Changes in v2, from Timur Tabi's and Zhi Wang's reviews, plus a couple of
small new ones at the end to do better printing of the new GMC API RPC
messages, below.

(Every number below is a v2 patch number. Patches 1 and 27 shifted the
numbering from v1. Patches 30 and 31 are new at the end).

* Patch 1 is new. The Rust PCI abstraction has no accessor for the PCI
  domain, which patch 23 needs.

* Patches 2 and 28 drop the generated bindings that nova-core never
  references, resulting in a net removal of 429 lines.

* Patch 4 documents the MCTP destination endpoint ID and version fields
  in fsp.rst.

* Patch 5 uses checked_add for the per-block DMA offsets.

* Patch 6 allocates each radix3 page table level zeroed at its
  page-rounded size, rather than appending entries and padding
  afterwards.

* Patch 7 wraps the two debug-log counters instead of trapping.

* Patch 8 drops the inline BLOB representation from the TLV payload
  loader, loses two FILE name checks that could not fire, returns
  ENODATA rather than EINVAL for a zero SIZE, and loads the GSP firmware
  image through the same loader instead of open-coding it.

* Patch 9 gives the log buffer a single const page-count parameter.

* Patch 11 corrects its commit message.

* Patch 15 asserts the GMC API header layout against GMCAPI_HEADER from
  the bindings, rather than against a hardcoded size.

* Patch 17 checks the MCTP version, the NVDM vendor ID and the declared
  element length, which is what Open RM checks on arriving elements.
  Patch 27 is new, and extends the same checks to the RPC receive path.

* Patch 18 requires the GMC header's payload size to match the length
  the transport header declares.

* Patches 20, 21 and 22 use expect rather than allow for
  clippy::too_many_arguments, or drop the attribute where the lint
  cannot fire.

* Patch 21 restores the FBIF aperture only once the loaded image halts,
  as Open RM does.

* Patch 23 sends the PCI location as domain, bus and device, which is
  what GSP-RM decodes, rather than the bus, device and function that
  PCI_DEVID packs.

* Patch 28 also removes the SetRegistry command, which the GSP_INIT
  payload replaced.

* Patch 30 is new. GMC debug lines printed a bare command id, so print
  the name next to it: GSP_INIT (0x10001).

* Patch 31 is new. GSP-initiated events number their sequence from
  0x8000000000000000, which printed as seq# 9223372036854775808, and
  the GSP_INIT reply was labeled an event. Print the sequence with that
  bit cleared, and label a reply as a response.

Nova-core boots the 570.144 GSP firmware today. This series moves the
driver to the r000 firmware ABI, which is stable across releases, so
firmware file names no longer carry a version number.

The firmware is not yet publicly available, so this series is for early
review, and should not be merged until the firmware ships.

Patch 26 makes the switch in one commit. Before that, everything works
on r570. After patch 26, only r000 is supported.

Beyond drm-rust-next, the series sits on these patchsets:

* The GIN interrupt controller series, which needs Danilo Krummrich's
  PCI and irq rework and Joel Fernandes' wait_for_completion_timeout().

* Eliot Courtney's NVKV encoder and decoder, which GSP_INIT is built on,
  and his Vec::push_init.

* Timur Tabi's "gpu: nova-core: write initial RPCs before booting
  GSP-RM", which patch 26 builds on.

* Timur Tabi's "rust: error: add remaining error codes", which supplies
  ENODATA and arrived in the 7.3 Rust pull as b93fb6e76ec1.

A branch with everything applied:

  https://github.com/johnhubbard/linux/tree/nova-core-run-on-r615-or-later-v2

Tested on TU117 (Turing, T400), GA104 (Ampere, RTX A4000), AD102 (Ada)
and GB202 (Blackwell, RTX PRO 6000), covering PCIe probe, chipset and
GPU name readback, the interrupt delivery self-test, and the KUnit
suites.

The vGPU team tested an earlier version of this series, minus the final
cleanups.


John Hubbard (31):
  rust: pci: add domain_nr() accessor
  gpu: nova-core: firmware: add r000 bindings
  gpu: nova-core: extract radix3 page table into its own module
  gpu: nova-core: set MCTP transport header version to 1
  gpu: nova-core: add Falcon helpers for r000 LOAD_EXEC events
  gpu: nova-core: zero-pad radix3 page table levels to page boundary
  gpu: nova-core: distinguish async GSP RPC traffic in debug logs
  gpu: nova-core: add optional ucodes firmware loading
  gpu: nova-core: add LIBOS3 log buffers and state monitor buffer
  gpu: nova-core: add build ID headers to debugfs log buffer dumps
  gpu: nova-core: rename the FbRanges elf field to fw_image
  gpu: nova-core: regs: add msgq v2 BAR0 register declarations
  gpu: nova-core: gsp: add msgq v2 internals
  gpu: nova-core: generalize allocate_command() for variable headers
  gpu: nova-core: add GMC API message types
  gpu: nova-core: add GMC send path
  gpu: nova-core: add GMC transport receive path
  gpu: nova-core: gsp: add GMC dispatch on receive
  gpu: nova-core: separate the generic falcon bootloader from FWSEC
  gpu: nova-core: handle the r000 load-and-execute HS binary event
  gpu: nova-core: handle the r000 load-and-execute bootloader event
  gpu: nova-core: gsp: add the GMC boot event dispatcher
  gpu: nova-core: gsp: add the GSP_INIT request builder
  gpu: nova-core: gsp: send GSP_INIT and decode its reply
  gpu: nova-core: gsp: pass the remaining log buffers to GSP-RM
  gpu: nova-core: switch to the r000 GSP firmware
  gpu: nova-core: gsp: validate RPC element framing on receive
  gpu: nova-core: gsp: remove the RPCs that GSP_INIT replaced
  gpu: nova-core: firmware: delete the r570 bindings
  gpu: nova-core: print GMC command names in debug logs
  gpu: nova-core: distinguish GMC event and response debug logs

 Documentation/gpu/nova/core/fsp.rst           |   2 +
 drivers/gpu/nova-core/driver.rs               |   2 +-
 drivers/gpu/nova-core/falcon.rs               | 166 +++-
 drivers/gpu/nova-core/fb.rs                   |  18 +-
 drivers/gpu/nova-core/fb/hal/gb100.rs         |  25 +-
 drivers/gpu/nova-core/fb/hal/gb202.rs         |   5 +-
 drivers/gpu/nova-core/firmware.rs             |  44 +-
 drivers/gpu/nova-core/firmware/bindata.rs     |  38 +
 .../nova-core/firmware/fwsec/bootloader.rs    | 112 +--
 .../gpu/nova-core/firmware/gen_bootloader.rs  | 215 +++++
 drivers/gpu/nova-core/firmware/gsp.rs         | 135 +--
 drivers/gpu/nova-core/firmware/radix3.rs      | 149 +++
 drivers/gpu/nova-core/firmware/tlv.rs         |  37 +
 drivers/gpu/nova-core/fsp.rs                  |   3 +-
 drivers/gpu/nova-core/gpu.rs                  |  23 +-
 drivers/gpu/nova-core/gsp.rs                  | 279 +++++-
 drivers/gpu/nova-core/gsp/boot.rs             | 550 ++++++++++-
 drivers/gpu/nova-core/gsp/cmdq.rs             | 829 +++++++++++-----
 .../gpu/nova-core/gsp/cmdq/continuation.rs    |   1 +
 drivers/gpu/nova-core/gsp/commands.rs         | 386 ++++----
 drivers/gpu/nova-core/gsp/fw.rs               | 899 ++++++++++--------
 drivers/gpu/nova-core/gsp/fw/commands.rs      | 275 +++---
 .../gsp/fw/{r570_144.rs => r000_00.rs}        |   2 +-
 .../gsp/fw/{r570_144 => r000_00}/bindings.rs  | 683 +++++--------
 drivers/gpu/nova-core/gsp/hal.rs              |  22 +-
 drivers/gpu/nova-core/gsp/hal/tu102.rs        |  18 +-
 drivers/gpu/nova-core/gsp/regs.rs             |  30 +
 drivers/gpu/nova-core/gsp/sequencer.rs        | 379 --------
 drivers/gpu/nova-core/irq/gsp.rs              |   2 +-
 drivers/gpu/nova-core/mctp.rs                 |  32 +-
 drivers/gpu/nova-core/regs.rs                 |   9 +-
 rust/helpers/pci.c                            |   5 +
 rust/kernel/pci.rs                            |  12 +
 33 files changed, 3217 insertions(+), 2170 deletions(-)
 create mode 100644 drivers/gpu/nova-core/firmware/bindata.rs
 create mode 100644 drivers/gpu/nova-core/firmware/gen_bootloader.rs
 create mode 100644 drivers/gpu/nova-core/firmware/radix3.rs
 rename drivers/gpu/nova-core/gsp/fw/{r570_144.rs => r000_00.rs} (95%)
 rename drivers/gpu/nova-core/gsp/fw/{r570_144 => r000_00}/bindings.rs (71%)
 delete mode 100644 drivers/gpu/nova-core/gsp/sequencer.rs

-- 
2.55.0

On Sat Aug 22, 2026 at 10:54 AM JST, John Hubbard wrote:
> Hi,
>
> I'm sending this just a few days after v1, which is soon, but Timur Tabi
> and Zhi Wang reviewed the whole series in that time, and that led to
> quite a lot of fixes and lines changed. So I think it is helpful to
> provide a v2 now, so that future reviewers can avoid rediscovering all
> those issues.

I only gave this a high-level look so far. This needs some reorganizing
before it can be properly reviewed. I think the size of the series can
be reduced by at least a third, maybe more.

First let's keep the series focused on r000: patches 7, 10, 30 and 31
are dealing with debug logs, not r000 proper. I'm sure they are useful
but they are technically a different topic from adding support for r000.

Then there is a lot of churn. For instance patch 28 removes 351 LoCs of
bindings introduced by patch 2. They are not required by the end of the
series, but kept as dead code after patch 26 because (IIUC) their RPCs
are removed later. This is one of the few cases where it is actually ok
to do two things in the same patch: were they removed during the switch
done by patch 26, their bindings wouldn't need to be introduced at all.
Actually that's how the sequencer is treated; I have many issues with
patch 26, but *that* one is ok - a bunch of deleted code is the simplest
thing to review.

The msgq v2 part also looks more messy than it needs to be. It tries to
keep both versions coexisting at the same time, even though only one of
them is ever used at any given time. I suspect it would be simpler to
implement the changes as part of the switch. Or at the very least, let's
make the cmdq generic if we want it to handle both, and place the
differences in sub-modules. This should also simplify the receive path
considerably.

Patch 26 does 5 different things over hundreds and hundreds of lines,
that's more contexts than a reviewer can hold in their head. This needs
to be split. I strongly suspect (more an intuition than a realization)
that a good part of it is actually intended to land in the
already-existing preceding patches. Other things can be simplified: for
instance `Bar0` arguments are added to many cmdq methods, which was
probably the only way of doing when the series was first developed, but
now we can store a `Bar0` in the relevant struct so let's do that.
Actually the `Cmdq` is probably overdue holding one.

I'll give a deeper look at the next revision, which I expect to be well
under the 3000 added LoCs (famous last words :)).

On Tue Sep 8, 2026 at 10:59 PM JST, Alexandre Courbot wrote:
> On Sat Aug 22, 2026 at 10:54 AM JST, John Hubbard wrote:
>> Hi,
>>
>> I'm sending this just a few days after v1, which is soon, but Timur Tabi
>> and Zhi Wang reviewed the whole series in that time, and that led to
>> quite a lot of fixes and lines changed. So I think it is helpful to
>> provide a v2 now, so that future reviewers can avoid rediscovering all
>> those issues.
>
> I only gave this a high-level look so far. This needs some reorganizing
> before it can be properly reviewed. I think the size of the series can
> be reduced by at least a third, maybe more.
>
> First let's keep the series focused on r000: patches 7, 10, 30 and 31
> are dealing with debug logs, not r000 proper. I'm sure they are useful
> but they are technically a different topic from adding support for r000.
>
> Then there is a lot of churn. For instance patch 28 removes 351 LoCs of
> bindings introduced by patch 2. They are not required by the end of the
> series, but kept as dead code after patch 26 because (IIUC) their RPCs
> are removed later. This is one of the few cases where it is actually ok
> to do two things in the same patch: were they removed during the switch
> done by patch 26, their bindings wouldn't need to be introduced at all.
> Actually that's how the sequencer is treated; I have many issues with
> patch 26, but *that* one is ok - a bunch of deleted code is the simplest
> thing to review.
>
> The msgq v2 part also looks more messy than it needs to be. It tries to
> keep both versions coexisting at the same time, even though only one of
> them is ever used at any given time. I suspect it would be simpler to
> implement the changes as part of the switch. Or at the very least, let's
> make the cmdq generic if we want it to handle both, and place the
> differences in sub-modules. This should also simplify the receive path
> considerably.
>
> Patch 26 does 5 different things over hundreds and hundreds of lines,
> that's more contexts than a reviewer can hold in their head. This needs
> to be split. I strongly suspect (more an intuition than a realization)
> that a good part of it is actually intended to land in the
> already-existing preceding patches. Other things can be simplified: for
> instance `Bar0` arguments are added to many cmdq methods, which was
> probably the only way of doing when the series was first developed, but
> now we can store a `Bar0` in the relevant struct so let's do that.
> Actually the `Cmdq` is probably overdue holding one.
>
> I'll give a deeper look at the next revision, which I expect to be well
> under the 3000 added LoCs (famous last words :)).

On a broader note, this series contains several chunks of functionality
that could be separate patch series and reviewed separately. I think
we'd be able to review this faster if it were explicitly split into
several series, as Alex noted there is some churn / patches that could
be put in a better order.

In particular, I see these sets of functionalities that could be done
separately and in order:
1. MCTP transport + cmdq transport
2. GMCAPI cmdq infra
3. Boot events for r000
4. Boot using r000 - that's systeminfo+staticinfo stuff
5. Migration to r000 + removal of r570. Ideally, the migration to r000
   can be the smallest commit to flip from (uses r570 for booth, doesn't
   use r000) => (doesn't use r570, uses r000).
6. Misc debug log stuff

On 9/8/26 10:47 PM, Eliot Courtney wrote:
> On Tue Sep 8, 2026 at 10:59 PM JST, Alexandre Courbot wrote:
>> On Sat Aug 22, 2026 at 10:54 AM JST, John Hubbard wrote:
>>> Hi,
>>>
>>> I'm sending this just a few days after v1, which is soon, but Timur Tabi
>>> and Zhi Wang reviewed the whole series in that time, and that led to
>>> quite a lot of fixes and lines changed. So I think it is helpful to
>>> provide a v2 now, so that future reviewers can avoid rediscovering all
>>> those issues.
>>
>> I only gave this a high-level look so far. This needs some reorganizing
>> before it can be properly reviewed. I think the size of the series can
>> be reduced by at least a third, maybe more.

Not according to my latest math. But I'll do what I can. :)

>>
>> First let's keep the series focused on r000: patches 7, 10, 30 and 31
>> are dealing with debug logs, not r000 proper. I'm sure they are useful
>> but they are technically a different topic from adding support for r000.
>>
>> Then there is a lot of churn. For instance patch 28 removes 351 LoCs of
>> bindings introduced by patch 2. They are not required by the end of the
>> series, but kept as dead code after patch 26 because (IIUC) their RPCs
>> are removed later. This is one of the few cases where it is actually ok
>> to do two things in the same patch: were they removed during the switch
>> done by patch 26, their bindings wouldn't need to be introduced at all.
>> Actually that's how the sequencer is treated; I have many issues with
>> patch 26, but *that* one is ok - a bunch of deleted code is the simplest
>> thing to review.

I was wondering about that approach to deleting the sequencer, so good to
hear that it is readable.

>>
>> The msgq v2 part also looks more messy than it needs to be. It tries to
>> keep both versions coexisting at the same time, even though only one of
>> them is ever used at any given time. I suspect it would be simpler to
>> implement the changes as part of the switch. Or at the very least, let's
>> make the cmdq generic if we want it to handle both, and place the
>> differences in sub-modules. This should also simplify the receive path
>> considerably.
>>
>> Patch 26 does 5 different things over hundreds and hundreds of lines,
>> that's more contexts than a reviewer can hold in their head. This needs
>> to be split. I strongly suspect (more an intuition than a realization)
>> that a good part of it is actually intended to land in the
>> already-existing preceding patches. Other things can be simplified: for
>> instance `Bar0` arguments are added to many cmdq methods, which was
>> probably the only way of doing when the series was first developed, but
>> now we can store a `Bar0` in the relevant struct so let's do that.
>> Actually the `Cmdq` is probably overdue holding one.
>>
>> I'll give a deeper look at the next revision, which I expect to be well
>> under the 3000 added LoCs (famous last words :)).
> 
> On a broader note, this series contains several chunks of functionality
> that could be separate patch series and reviewed separately. I think
> we'd be able to review this faster if it were explicitly split into
> several series, as Alex noted there is some churn / patches that could
> be put in a better order.

OK, I'll work on splitting up this huge series into digestible smaller
patchsets.

> 
> In particular, I see these sets of functionalities that could be done
> separately and in order:
> 1. MCTP transport + cmdq transport
> 2. GMCAPI cmdq infra
> 3. Boot events for r000
> 4. Boot using r000 - that's systeminfo+staticinfo stuff
> 5. Migration to r000 + removal of r570. Ideally, the migration to r000
>    can be the smallest commit to flip from (uses r570 for booth, doesn't
>    use r000) => (doesn't use r570, uses r000).
> 6. Misc debug log stuff

thanks,
-- 
John Hubbard