Skip to content

virtio-devices: Fix cap_len for VIRTIO_PCI_CAP_PCI_CFG - #8238

Merged
rbradford merged 4 commits into
cloud-hypervisor:mainfrom
weltling:virtio-pci-cfg-cap-len
May 18, 2026
Merged

virtio-devices: Fix cap_len for VIRTIO_PCI_CAP_PCI_CFG#8238
rbradford merged 4 commits into
cloud-hypervisor:mainfrom
weltling:virtio-pci-cfg-cap-len

Conversation

@weltling

Copy link
Copy Markdown
Member

The VIRTIO_PCI_CAP_PCI_CFG capability was emitted with cap_len 16 instead of 20. With this change cap_len reflects the full capability size, including the trailing pci_cfg_data window.

Per virtio 1.2 section 4.1.4.9, the PCI configuration access capability is a 16 byte virtio_pci_cap followed by a 4 byte pci_cfg_data window, so its declared cap_len must be 20.

weltling added 4 commits May 16, 2026 15:41
VirtioPciCfgCap::new built its inner header via VirtioPciCap::new,
which sized cap_len from the bare virtio_pci_cap layout, yielding
16. The emitted capability is VirtioPciCfgCap, which appends a four
byte pci_cfg_data window, so the correct value is 20.

The virtio 1.2 specification defines this cap as virtio_pci_cap
followed by pci_cfg_data[4] and requires cap_len to
cover the whole structure. Build the header inline so cap_len
reflects the actual emitted size, matching VirtioPciNotifyCap and
VirtioPciCap64.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Regression test for the cap_len fix. The emitted VirtioPciCfgCap
must report cap_len 20, covering the trailing pci_cfg_data window
per virtio 1.2 section 4.1.4.9.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Assert the emitted VirtioPciCfgCap carries cfg_type 5, the value
assigned to PciCapabilityType::Pci by virtio 1.2 section 4.1.4.1.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Assert VirtioPciNotifyCap and VirtioPciCap64 size cap_len from
their own type. Catches a future regression of the same shape as
the VirtioPciCfgCap one in any of the sibling capabilities.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
@weltling
weltling requested a review from a team as a code owner May 16, 2026 14:23

@phip1611 phip1611 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

General question: How could the old behavior fail with a Linux or Windows guest? What is a typical failure one would see/expect?

@rbradford
rbradford added this pull request to the merge queue May 18, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 18, 2026
@rbradford
rbradford added this pull request to the merge queue May 18, 2026
@weltling

Copy link
Copy Markdown
Member Author

LGTM, thanks!

General question: How could the old behavior fail with a Linux or Windows guest? What is a typical failure one would see/expect?

I dug into the driver code and no guest actually uses VIRTIO_PCI_CAP_PCI_CFG. Linux and Windows virtio driver both skip it and map BARs directly. However, QEMU carries a full spec compliance, same as this PR targets. This is a correctness fix for a field in config space. I think some early boot situations that cannot use MMIO could be affected, like firmware.

Thanks

Merged via the queue into cloud-hypervisor:main with commit 2c86362 May 18, 2026
41 checks passed
@github-project-automation github-project-automation Bot moved this from 🆕 New to ✅ Done in Cloud Hypervisor Roadmap Jul 8, 2026
@rbradford rbradford added the bug-fix Bug fix to include in release notes label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix Bug fix to include in release notes

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants