Backport DirectNIC patches to nvidia_stable-11.0 from nvidia_stable-10.0

[tdavenvidia]

This PR contains GPA=HPA and ACS changes required for DirectNIC GB300 w/CX8.

[nvmochs]

@tdavenvidia Some comments from my review...

---

9170f9d NVIDIA: SAUCE: hw/arm: GB300 workaround for GPU BAR1 GPA=HPA

This patch is missing a pick tag.

---

All of the patches seem to have the pick tags in the wrong order, also the SHAs use an inconsistent format. Please pick with -x -s.

Example of how it should flow for a clean picked patch:

<existing trailers from the original patch>
(cherry picked from commit <full SHA> https://github.com/NVIDIA/QEMU/commits/nvidia_stable-10.1)
Signed-off-by: <your details>

Example of how it should flow for a patch that required fixup:

<existing trailers from the original patch>
(backported from commit <full SHA> https://github.com/NVIDIA/QEMU/commits/nvidia_stable-10.1)
[tdave: details about what needed to be changed]
Signed-off-by: <your details>

---

Some functional findings from Codex review. Codex acknowledged these were present in the patches on the 10.1 branch, but we should review and address them now (if applicable).

2. Potential crash with fd-based VFIO devices in 6d03950.

   hw/arm/virt-acpi-build.c:277 assumes vbasedev.sysfsdev exists and passes the result of realpath() directly to fopen() at hw/arm/virt-acpi-build.c:285. QEMU supports VFIO devices supplied by fd without sysfsdev, so this can reach fopen(NULL, ...). It should reject/skip a missing sysfsdev or failed realpath().

3. Possible infinite loop during ROM allocation in 6d03950.

   In hw/arm/virt-acpi-build.c:511, if a ROM needs relocation but nvidia_find_mmio() returns false, nothing changes before the unconditional retry. The same ROM is then selected forever, potentially hanging ACPI table construction when the aperture is full.

4. 32-bit prefetchable BARs can be corrupted by 6d03950.

   The loop at hw/arm/virt-acpi-build.c:312 checks only IORESOURCE_PREFETCH, but then treats every matching BAR as 64-bit and writes its following DWORD. For a 32-bit prefetchable BAR, that DWORD belongs to the next BAR. IORESOURCE_MEM_64 is defined by the patch but never checked.

5. ACS Control is not read-only when acs-caps=0.

   hw/pci/pcie.c:1299 clears the write mask only when p->acs_caps is nonzero. Consequently, an explicit zero configuration can still be overwritten by the guest, contrary to the stated goal of preserving the hypervisor configuration. If backward compatibility requires the default to remain writable, a separate “configured/lock” flag is needed to distinguish default zero from explicit zero.

[tdavenvidia]

@nvmochs Thank you for the review. I appreciate it.

While the functional findings from Codex especially 2, 3 ,4 are valid, we have had never encounter them likely because we wont touch 32bit BAR and ROM BAR path for DirectNIC.

Having said that, I am addressing all of the comments and apply the required fix. I will push new changes soon.

[nirmoy]

BaseOS Kernel Review

Warning

⚠️ Review needs attention

Grace PCIe identity mapping can relocate BARs to unreachable or unmapped GPAs and extend bridge windows over existing PCI resources. ARM virt builds without VFIO-PCI can also fail to link.

Findings: Critical 0 · High 4 · Medium 6 · Low 2

🔍 Review artifacts

• 🟠 Boro review: 12 findings

Review metadata

• Reviewed head: 151eb2e7556c
• Overall status: attention needed

This comment is maintained by BaseOS Reviewer and updated when the GitHub watcher publishes a newer review.

[nvmochs]

@nvmochs Thank you for the review. I appreciate it.

While the functional findings from Codex especially 2, 3 ,4 are valid, we have had never encounter them likely because we wont touch 32bit BAR and ROM BAR path for DirectNIC.

Having said that, I am addressing all of the comments and apply the required fix. I will push new changes soon.

Sounds good, please ping when this is ready for re-review. Thanks!

[tdavenvidia]

@nvmochs I already did yesterday, but BORO had some comments which I am replying here:

Thanks to BORO's Review of PR #35. Here is my reasoning. I will be pushing changes soon.

1. nvidia_dev_shift_mmio64() masks a BAR with PCI_BASE_ADDRESS_MEM_MASK, allegedly clearing the upper 32 bits (High). PCI_BASE_ADDRESS_MEM_MASK is (~0x0fUL). On 64-bit Linux, unsigned long is 64 bits, so this mask only clears the low 4 flag bits, not the high address bits. False positive, closed.

2. Sysfs resource parser truncates BAR addresses via %lx into a uint64_t* on a 32-bit host (High). Same root cause as Directnic GPA=HPA #1 — on 64-bit Linux, unsigned long and uint64_t are the same width, so the format specifier correctly matches. False positive, closed.

3. nvidia_find_mmio()'s fallback window extension has no overlap check, per the code's own /* TODO: check conflicts with the extended window */ (High). This fallback path only triggers when a bridge's total downstream BAR footprint can't find a clear gap in its parent's window because that window is fragmented by other devices. DirectNIC requires each GPU and its DMA-PF to sit under the same PCIe switch, mirroring bare metal, which keeps that hierarchy sparse by design. The parent window is also enormous (256TB with grace-pcie-mmio-identity=on) against a GPU's ~1TB footprint. Structurally can't be hit under DirectNIC's topology. The TODO is an intentional hint for a future, non-DirectNIC use of this mechanism. Not fixing.

4. A 47-bit PCI aperture gets installed regardless of the actual CPU's physical-address width, and highest_gpa is allegedly set in a way validation can't catch (High). The pa_bits value fed into this logic is max_vm_pa_size, sourced from kvm_arm_get_max_vm_ipa_size() — the real, host-queried KVM IPA limit. The aperture size and the later validation both derive from that same value, self-consistent by construction for KVM, the only accelerator used here. False positive, closed.

5. The new ACS Control write-mask change breaks migration from an older QEMU (High). Migration is not supported with DirectNIC. Ruled out.

6. CONFIG_ARM_VIRT=y, CONFIG_ACPI=y, CONFIG_VFIO_PCI=n can fail to link (Medium). This feature exists only to support vfio-pci passthrough — no real product build of this fork would ever disable CONFIG_VFIO_PCI. Not worth fixing.

7. The overlap-avoidance loop in fix_pci_bar_GB200_nvidia() doesn't pin BAR1, so a BAR0/BAR1 overlap could in theory relocate BAR1 itself (Medium). BAR0 and BAR1 are read directly from the real host as genuine physical BARs, and by hardware reality they cannot overlap each other. This mechanism is also being replaced by the RFC v2 firmware-based redesign. Not a real bug.

8. grace-pcie-mmio-identity=on with ACPI disabled changes the memory map but never actually runs the GPA=HPA BAR setup, since that logic is only wired into the ACPI table-build path (Medium). This property is only ever enabled for ACPI-based boot, per the Grace virt guide. Non-ACPI/FDT boot is not a supported path for this feature. Moot, not fixing.

9. acs-caps=0xFFFF is used as the "unset" sentinel (ACS_CAP_UNDEFINED), but it's also a value a user could legitimately pass explicitly — and now gets silently treated as "not configured" instead of correctly failing validation the way it used to (Medium). Every call site does p->acs_caps == ACS_CAP_UNDEFINED ? 0 : p->acs_caps, so an explicit acs-caps=0xffff is indistinguishable from unset. Real regression in validation behavior. Legit, needs a fix.

10. ACS validation failure leaks the AER log that was already allocated (Medium). pcie_aer_init() allocates dev->exp.aer_log.log before pcie_acs_init() runs, and the error path skips pcie_aer_exit(). Every ACS-bearing device in these configs is created via -device at QEMU launch, never hotplugged, and a -device realize failure at startup is fatal — QEMU exits the whole process right then. The leak can never accumulate, and the OS reclaims everything on exit. Zero practical impact, not fixing.

11. Adding the new ACS extended capability breaks migration from an older QEMU (Medium). Migration is not supported with DirectNIC. Ruled out.

12. Comment typo "avoind" should be "avoid", and the comment exceeds preferred line length (Low). Legit, will fix.

13. An added if condition exceeds QEMU's ~80-column line-length convention; the shared vendor-ID check could also be factored out for readability (Low). Legit, will fix.

14. A continued macro line uses tab indentation instead of 4 spaces (Low). Legit, will fix.

15. Commit message grammar: "possible however" should have punctuation, "resembles to baremetal" should be "resembles bare metal", "CX8 NIC(the DMA-PF)" needs spacing (Low). Legit, will fix.

16. Commit message: "is real BAR exposed to VM" should be "is a real BAR exposed to the VM" (Low). Legit, will fix.

17. Commit message: "Simialr" is a typo for "Similar", and "requires workaround" is missing an article (Low). Legit, will fix.

18. Commit message: "PASID capable" should be hyphenated, "for example Nvidia GPU" needs an article, "usecase" should be "use case" (Low). Legit, will fix.

19. Commit message typo: "uisng" should be "using" (Low). Legit, will fix.

20. Final low-severity wording nit, same cleanup bucket as 12-19 (Low). Legit, will fix.

-Tushar

[tdavenvidia]

@nvmochs

I pushed the new changes; addressed all BORO review as well as your review comments. Please take a look.

[nvmochs]

@tdavenvidia Some additional review comments...

While the provenance now looks consistent, is still not correct. The "new" items (tag, annotation notes if needed, and your sign-off) should come after the existing trailers.

So using "NVIDIA: SAUCE: hw/arm: GB200 DirectNIC GPA=HPA” as an example, I would expect to see the provenance trailers look like this:

Signed-off-by: Shanker Donthineni <[email protected]>
Signed-off-by: Tushar Dave <[email protected]>
Signed-off-by: Matthew R. Ochs <[email protected]>
Acked-by: Matthew R. Ochs <[email protected]>
Acked-by: Mitchell Augustin <[email protected]>
Signed-off-by: Matthew R. Ochs <[email protected]>
(backported from commit 31a853dfa550a37b5b68a3d8683ba248348a04ba https://github.com/NVIDIA/QEMU/commits/nvidia_stable-10.1)
[tdave: context adjustment in hw/arm/virt-acpi-build.c build_dsdt() due to
 VFIO_PCI_BASE renamed to VFIO_PCI_DEVICE and build_dsdt signature change;
 context adjustment in include/hw/arm/virt.h due to new fields added in
 11.0; added NULL checks for sysfsdev/realpath and a missing break in the
 ROM-relocation retry loop, prevent 32-bit prefetchable BARs corruption
 per PR review feedback]
Signed-off-by: Tushar Dave <[email protected]>

---

There were several complaints about checkpatch warnings / errors. Some of these existed last time, some are new. If possible, I think we should just address them now so we have "clean" patches.

These are the exact diagnostics. Line numbers are from each commit’s post-image, so later commits may shift final-tree lines.

  ### 8283f9feee78 — 1 error, 8 warnings

  Error:

  - Missing braces around if (!ncfg->rsize) in hw/arm/virt-acpi-build.c:481.

  Warnings, all lines over 80 characters:

  - Bridge base/limit assignments at lines 188–190.
  - nvidia_update_bridge_window() declaration at line 194.
  - nvidia_dev_unassigned_mmio64() declaration at line 532.
  - build_dsdt() declaration at line 1729.
  - acpi_dsdt_add_pci() call at line 1763.
  - virt_acpi_build() declaration at line 1860.

  ### 96a7dd081135 — 6 errors, 1 warning

  Errors:

  - Missing braces around three if statements at lines 232, 248, and 252.
  - Missing space in if(ranges_overlap(...)) at line 255.
  - The same ranges_overlap() line exceeds 90 characters.
  - The QEMU_ALIGN_UP() assignment at line 256 exceeds 90 characters.

  Warning:

  - The QEMU_ALIGN_UP() assignment at line 235 exceeds 80 characters.

  ### 3e0ee1b23761 — 1 error, 4 warnings

  Error:

  - error_setg() has one extra leading space, producing suspect conditional indentation at lines 1288–1289.

  Warnings, all lines over 80 characters:

  - pcie_acs_init() declaration in hw/pci/pcie.c:1263.
  - Both lines of the error_setg() call at lines 1289–1290.
  - pcie_acs_init() declaration in include/hw/pci/pcie.h:140.

  ### ef7ba6ca3141 — 1 error, 3 warnings

  Error:

  - Checkpatch again reports the incorrectly indented error_setg() because this commit modifies its surrounding condition. The bad indentation originated in 3e0ee1b23761; ef7ba6… exposes it again rather than
    introducing it.

  Warnings:

  - Long pcie_acs_init() argument in pcie_root_port.c:123.
  - Long pcie_acs_init() argument in xio3130_downstream.c:119.
  - Long ACS Control comment in pcie.c:1300.

  Additionally, checkpatch does not detect the newly added over-indentation here:

  } else {
        break;
  }

  The break at hw/arm/virt-acpi-build.c:538 should use the normal four-space nesting:

  } else {
      break;
  }

---

ef7ba6c has an inaccurate backport annotation.

 It says:

 [tdave: renamed ACS_CAP_UNDEFINED to ACS_CAP_NOT_CONFIGURED ...]

 ACS_CAP_UNDEFINED does not exist in source commit b1bcf26524de or its parent. The actual deviation is introducing a sentinel to distinguish omitted acs-caps from explicit zero. The note should describe
 that directly.

---

Codex stated:

acs-caps uses the valid uint16 value 0xffff as the “not configured” sentinel. As a result, explicitly specifying acs-caps=0xffff is indistinguishable from omitting the property: validation is skipped, ACS Control is set to zero, and guest writes remain enabled. Track whether the property was explicitly configured separately instead of using an in-range sentinel.

Should we catch 0xffff as an invalid value and fail early?

[tdavenvidia]

@nvmochs

Addressed all the comments from your last review.

Trailer order is fixed on all 6 commits — provenance trailers, then (backported from ...), then the [tdave: ...] note, then my sign-off last. The backport annotation on the ACS read-only commit now describes the actual deviation directly instead of naming a symbol that doesn't exist in the source commit. All the checkpatch errors/warnings you listed are cleared, including the over-indented break in hw/arm/virt-acpi-build.c that checkpatch itself didn't catch.

On the acs-caps=0xffff question from Codex: the root issue was that acs-caps used a same-width sentinel value (0xFFFF) to represent "not configured," so an explicit acs-caps=0xffff was indistinguishable from the property never being touched — it silently fell through to the unconfigured path instead of being validated. I changed the property to use a custom setter that tracks whether acs-caps was actually set by the user, independent of what value was set, so there's no value collision possible anymore. acs-caps=0xffff (and any value with bits outside the supported ACS mask) is now rejected the same way any other invalid value is — Unsupported ACS capabilities 0x... were supplied. Supported capabilities are 0x5f. An explicit acs-caps=0 is also now correctly distinguished from "not configured" and gets the read-only lockdown as expected.

Verified with checkpatch (clean across all 6 commits) and a real build + launch matrix covering every value class — valid subsets, invalid in-range masks, and out-of-16-bit-range inputs (all correctly rejected at parse time now) — on both root ports and downstream ports, plus confirmed the configured value survives a device reset correctly.

[nvmochs]

@nvmochs

Addressed all the comments from your last review.

Trailer order is fixed on all 6 commits — provenance trailers, then (backported from ...), then the [tdave: ...] note, then my sign-off last. The backport annotation on the ACS read-only commit now describes the actual deviation directly instead of naming a symbol that doesn't exist in the source commit. All the checkpatch errors/warnings you listed are cleared, including the over-indented break in hw/arm/virt-acpi-build.c that checkpatch itself didn't catch.

On the acs-caps=0xffff question from Codex: the root issue was that acs-caps used a same-width sentinel value (0xFFFF) to represent "not configured," so an explicit acs-caps=0xffff was indistinguishable from the property never being touched — it silently fell through to the unconfigured path instead of being validated. I changed the property to use a custom setter that tracks whether acs-caps was actually set by the user, independent of what value was set, so there's no value collision possible anymore. acs-caps=0xffff (and any value with bits outside the supported ACS mask) is now rejected the same way any other invalid value is — Unsupported ACS capabilities 0x... were supplied. Supported capabilities are 0x5f. An explicit acs-caps=0 is also now correctly distinguished from "not configured" and gets the read-only lockdown as expected.

Verified with checkpatch (clean across all 6 commits) and a real build + launch matrix covering every value class — valid subsets, invalid in-range masks, and out-of-16-bit-range inputs (all correctly rejected at parse time now) — on both root ports and downstream ports, plus confirmed the configured value survives a device reset correctly.

Thanks @tdavenvidia!

No further issues from me.

Acked-by: Matthew R. Ochs <[email protected]>

[nvmochs]

@shamiali2008 Can you review this series as well?

[shamiali2008]

LGTM:
Acked-by: Shameer Kolothum [email protected]