From 6fcf5ebafad65adc19a616260ca7dc90005785d1 Mon Sep 17 00:00:00 2001 From: Jonah Palmer Date: Mon, 21 Jul 2025 15:02:08 +0000 Subject: [PATCH 01/17] virtio: fix off-by-one and invalid access in virtqueue_ordered_fill Commit b44135daa372 introduced virtqueue_ordered_fill for VIRTIO_F_IN_ORDER support but had a few issues: * Conditional while loop used 'steps <= max_steps' but should've been 'steps < max_steps' since reaching steps == max_steps would indicate that we didn't find an element, which is an error. Without this change, the code would attempt to read invalid data at an index outside of our search range. * Incremented 'steps' using the next chain's ndescs instead of the current one. This patch corrects the loop bounds and synchronizes 'steps' and index increments. We also add a defensive sanity check against malicious or invalid descriptor counts to avoid a potential infinite loop and DoS. Fixes: b44135daa372 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support") Reported-by: terrynini Signed-off-by: Jonah Palmer Message-Id: <20250721150208.2409779-1-jonah.palmer@oracle.com> Reviewed-by: Si-Wei Liu Acked-by: Jason Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 2ab1d20769..9a81ad912e 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -938,18 +938,18 @@ static void virtqueue_packed_fill(VirtQueue *vq, const VirtQueueElement *elem, static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem, unsigned int len) { - unsigned int i, steps, max_steps; + unsigned int i, steps, max_steps, ndescs; i = vq->used_idx % vq->vring.num; steps = 0; /* - * We shouldn't need to increase 'i' by more than the distance - * between used_idx and last_avail_idx. + * We shouldn't need to increase 'i' by more than or equal to + * the distance between used_idx and last_avail_idx (max_steps). */ max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num; /* Search for element in vq->used_elems */ - while (steps <= max_steps) { + while (steps < max_steps) { /* Found element, set length and mark as filled */ if (vq->used_elems[i].index == elem->index) { vq->used_elems[i].len = len; @@ -957,8 +957,18 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem, break; } - i += vq->used_elems[i].ndescs; - steps += vq->used_elems[i].ndescs; + ndescs = vq->used_elems[i].ndescs; + + /* Defensive sanity check */ + if (unlikely(ndescs == 0 || ndescs > vq->vring.num)) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: %s invalid ndescs %u at position %u\n", + __func__, vq->vdev->name, ndescs, i); + return; + } + + i += ndescs; + steps += ndescs; if (i >= vq->vring.num) { i -= vq->vring.num; From c1997099dc26d95eb9f2249f2894aabf4cf0bf8b Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Thu, 24 Jul 2025 14:59:27 +0200 Subject: [PATCH 02/17] vhost: Do not abort on log-start error Commit 3688fec8923 ("memory: Add Error** argument to .log_global_start() handler") enabled vhost_log_global_start() to return a proper error, but did not change it to do so; instead, it still aborts the whole process on error. This crash can be reproduced by e.g. killing a virtiofsd daemon before initiating migration. In such a case, qemu should not crash, but just make the attempted migration fail. Buglink: https://issues.redhat.com/browse/RHEL-94534 Reported-by: Tingting Mao Signed-off-by: Hanna Czenczek Message-Id: <20250724125928.61045-2-hreitz@redhat.com> Reviewed-by: Manos Pitsidianakis Reviewed-by: Stefano Garzarella Tested-by: Lei Yang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index c30ea1156e..05ad5de629 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1110,7 +1110,8 @@ static bool vhost_log_global_start(MemoryListener *listener, Error **errp) r = vhost_migration_log(listener, true); if (r < 0) { - abort(); + error_setg_errno(errp, -r, "vhost: Failed to start logging"); + return false; } return true; } From d63c388dadb7717f6391e1bb7f11728c0c1a3e36 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Thu, 24 Jul 2025 14:59:28 +0200 Subject: [PATCH 03/17] vhost: Do not abort on log-stop error Failing to stop logging in a vhost device is not exactly fatal. We can log such an error, but there is no need to abort the whole qemu process because of it. Signed-off-by: Hanna Czenczek Message-Id: <20250724125928.61045-3-hreitz@redhat.com> Reviewed-by: Manos Pitsidianakis Reviewed-by: Stefano Garzarella Tested-by: Lei Yang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 05ad5de629..6557c58d12 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1122,7 +1122,8 @@ static void vhost_log_global_stop(MemoryListener *listener) r = vhost_migration_log(listener, false); if (r < 0) { - abort(); + /* Not fatal, so report it, but take no further action */ + warn_report("vhost: Failed to stop logging"); } } From 6071d13c6a37493a6b26e1609b09a98aa058038a Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 27 Jul 2025 15:22:36 +0900 Subject: [PATCH 04/17] virtio-net: Fix VLAN filter table reset timing Problem ------- The expected initial state of the table depends on feature negotiation: With VIRTIO_NET_F_CTRL_VLAN: The table must be empty in accordance with the specification. Without VIRTIO_NET_F_CTRL_VLAN: The table must be filled to permit all VLAN traffic. Prior to commit 06b636a1e2ad ("virtio-net: do not reset vlan filtering at set_features"), virtio_net_set_features() always reset the VLAN table. That commit changed the behavior to skip table reset when VIRTIO_NET_F_CTRL_VLAN was negotiated, assuming the table would be properly cleared during device reset and remain stable. However, this assumption breaks when a driver renegotiates features: 1. Initial negotiation without VIRTIO_NET_F_CTRL_VLAN (table filled) 2. Renegotiation with VIRTIO_NET_F_CTRL_VLAN (table will not be cleared) The problem was exacerbated by commit 0caed25cd171 ("virtio: Call set_features during reset"), which triggered virtio_net_set_features() during device reset, exposing the bug whenever VIRTIO_NET_F_CTRL_VLAN was negotiated after a device reset. Solution -------- Fix the issue by initializing the table when virtio_net_set_features() is called to change the VIRTIO_NET_F_CTRL_VLAN bit of vdev->guest_features. This approach ensures the correct table state regardless of feature negotiation sequence by performing initialization in virtio_net_set_features() as QEMU did prior to commit 06b636a1e2ad ("virtio-net: do not reset vlan filtering at set_features"). This change still preserves the goal of the commit, which was to avoid resetting the table during migration, by checking whether the VIRTIO_NET_F_CTRL_VLAN bit of vdev->guest_features is being changed; vdev->guest_features is set before virtio_net_set_features() gets called during migration. It also avoids resetting the table when the driver sets a feature bitmask with no change for the VIRTIO_NET_F_CTRL_VLAN bit, which makes the operation idempotent and its semantics cleaner. Additionally, this change ensures the table is initialized after feature negotiation and before the DRIVER_OK status bit being set for compatibility with the Linux driver before commit 50c0ada627f5 ("virtio-net: fix race between ndo_open() and virtio_device_ready()"), which did not ensure to set the DRIVER_OK status bit before modifying the table. Fixes: 06b636a1e2ad ("virtio-net: do not reset vlan filtering at set_features") Cc: qemu-stable@nongnu.org Reported-by: Konstantin Shkolnyy Signed-off-by: Akihiko Odaki Tested-by: Konstantin Shkolnyy Tested-by: Lei Yang Message-Id: <20250727-vlan-v3-1-bbee738619b1@rsg.ci.i.u-tokyo.ac.jp> Tested-by: Konstantin Shkolnyy Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index c4c49b0f9c..6b5b5dace3 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -929,8 +929,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) vhost_net_save_acked_features(nc->peer); } - if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { - memset(n->vlans, 0xff, MAX_VLAN >> 3); + if (virtio_has_feature(vdev->guest_features ^ features, VIRTIO_NET_F_CTRL_VLAN)) { + bool vlan = virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN); + memset(n->vlans, vlan ? 0 : 0xff, MAX_VLAN >> 3); } if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) { @@ -3942,6 +3943,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->mac_table.macs = g_malloc0(MAC_TABLE_ENTRIES * ETH_ALEN); n->vlans = g_malloc0(MAX_VLAN >> 3); + memset(n->vlans, 0xff, MAX_VLAN >> 3); nc = qemu_get_queue(n->nic); nc->rxfilter_notify_enabled = 1; @@ -4041,7 +4043,6 @@ static void virtio_net_reset(VirtIODevice *vdev) memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac)); qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac); - memset(n->vlans, 0, MAX_VLAN >> 3); /* Flush any async TX */ for (i = 0; i < n->max_queue_pairs; i++) { From cad9aa6fbdccd95e56e10cfa57c354a20a333717 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 27 Jul 2025 15:50:08 +0900 Subject: [PATCH 05/17] pcie_sriov: Fix configuration and state synchronization Fix issues in PCIe SR-IOV configuration register handling that caused inconsistent internal state due to improper write mask handling and incorrect migration behavior. Two main problems were identified: 1. VF Enable bit write mask handling: pcie_sriov_config_write() incorrectly assumed that its val parameter was already masked, causing it to ignore the actual write mask. This led to the VF Enable bit being processed even when masked, resulting in incorrect VF registration/unregistration. It is identified as CVE-2025-54567. 2. Migration state inconsistency: pcie_sriov_pf_post_load() unconditionally called register_vfs() regardless of the VF Enable bit state, creating inconsistent internal state when VFs should not be enabled. Additionally, it failed to properly update the NumVFs write mask based on the current configuration. It is identified as CVE-2025-54566. Root cause analysis revealed that both functions relied on incorrect special-case assumptions instead of properly reading and consuming the actual configuration values. This change introduces a unified consume_config() function that reads actual configuration values and synchronize the internal state without special-case assumptions. The solution only adds register read overhead in non-hot-path code while ensuring correct SR-IOV state management across configuration writes and migration scenarios. Fixes: 5e7dd17e4348 ("pcie_sriov: Remove num_vfs from PCIESriovPF") Fixes: f9efcd47110d ("pcie_sriov: Register VFs after migration") Fixes: CVE-2025-54566 Fixes: CVE-2025-54567 Cc: qemu-stable@nongnu.org Reported-by: Corentin BAYET Signed-off-by: Akihiko Odaki Message-Id: <20250727-wmask-v2-1-394910b1c0b6@rsg.ci.i.u-tokyo.ac.jp> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie_sriov.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index 3ad18744f4..8a4bf0d6f7 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -64,6 +64,27 @@ static void unregister_vfs(PCIDevice *dev) pci_set_word(dev->wmask + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0xffff); } +static void consume_config(PCIDevice *dev) +{ + uint8_t *cfg = dev->config + dev->exp.sriov_cap; + + if (pci_get_word(cfg + PCI_SRIOV_CTRL) & PCI_SRIOV_CTRL_VFE) { + register_vfs(dev); + } else { + uint8_t *wmask = dev->wmask + dev->exp.sriov_cap; + uint16_t num_vfs = pci_get_word(cfg + PCI_SRIOV_NUM_VF); + uint16_t wmask_val = PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI; + + unregister_vfs(dev); + + if (num_vfs <= pci_get_word(cfg + PCI_SRIOV_TOTAL_VF)) { + wmask_val |= PCI_SRIOV_CTRL_VFE; + } + + pci_set_word(wmask + PCI_SRIOV_CTRL, wmask_val); + } +} + static bool pcie_sriov_pf_init_common(PCIDevice *dev, uint16_t offset, uint16_t vf_dev_id, uint16_t init_vfs, uint16_t total_vfs, uint16_t vf_offset, @@ -416,30 +437,13 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, trace_sriov_config_write(dev->name, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn), off, val, len); - if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) { - if (val & PCI_SRIOV_CTRL_VFE) { - register_vfs(dev); - } else { - unregister_vfs(dev); - } - } else if (range_covers_byte(off, len, PCI_SRIOV_NUM_VF)) { - uint8_t *cfg = dev->config + sriov_cap; - uint8_t *wmask = dev->wmask + sriov_cap; - uint16_t num_vfs = pci_get_word(cfg + PCI_SRIOV_NUM_VF); - uint16_t wmask_val = PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI; - - if (num_vfs <= pci_get_word(cfg + PCI_SRIOV_TOTAL_VF)) { - wmask_val |= PCI_SRIOV_CTRL_VFE; - } - - pci_set_word(wmask + PCI_SRIOV_CTRL, wmask_val); - } + consume_config(dev); } void pcie_sriov_pf_post_load(PCIDevice *dev) { if (dev->exp.sriov_cap) { - register_vfs(dev); + consume_config(dev); } } From 96c75abc872c1a90c8e1f4a4d2ed35e89a1befe9 Mon Sep 17 00:00:00 2001 From: Sairaj Kodilkar Date: Fri, 1 Aug 2025 11:35:02 +0530 Subject: [PATCH 06/17] hw/i386/amd_iommu: Fix MMIO register write tracing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Define separate functions to trace MMIO write accesses instead of using `trace_amdvi_mmio_read()` for both read and write. Signed-off-by: Sairaj Kodilkar Reviewed-by: Vasant Hegde Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20250801060507.3382-2-sarunkod@amd.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 5a24c17548..7fb0bb68f0 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -592,18 +592,31 @@ static void amdvi_cmdbuf_run(AMDVIState *s) } } -static void amdvi_mmio_trace(hwaddr addr, unsigned size) +static inline uint8_t amdvi_mmio_get_index(hwaddr addr) { uint8_t index = (addr & ~0x2000) / 8; if ((addr & 0x2000)) { /* high table */ index = index >= AMDVI_MMIO_REGS_HIGH ? AMDVI_MMIO_REGS_HIGH : index; - trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07); } else { index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index; - trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07); } + + return index; +} + +static void amdvi_mmio_trace_read(hwaddr addr, unsigned size) +{ + uint8_t index = amdvi_mmio_get_index(addr); + trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07); +} + +static void amdvi_mmio_trace_write(hwaddr addr, unsigned size, uint64_t val) +{ + uint8_t index = amdvi_mmio_get_index(addr); + trace_amdvi_mmio_write(amdvi_mmio_low[index], addr, size, val, + addr & ~0x07); } static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr, unsigned size) @@ -623,7 +636,7 @@ static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr, unsigned size) } else if (size == 8) { val = amdvi_readq(s, addr); } - amdvi_mmio_trace(addr, size); + amdvi_mmio_trace_read(addr, size); return val; } @@ -770,7 +783,7 @@ static void amdvi_mmio_write(void *opaque, hwaddr addr, uint64_t val, return; } - amdvi_mmio_trace(addr, size); + amdvi_mmio_trace_write(addr, size, val); switch (addr & ~0x07) { case AMDVI_MMIO_CONTROL: amdvi_mmio_reg_write(s, size, val, addr); From 47d50cc421b832650822d73431d920bb8a80bc38 Mon Sep 17 00:00:00 2001 From: Sairaj Kodilkar Date: Fri, 1 Aug 2025 11:35:03 +0530 Subject: [PATCH 07/17] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ats_enabled field is set using HTTUNEN, which is wrong. Fix this by removing the field as it is never used. MST: includes a tweak suggested by Philippe Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU") Signed-off-by: Sairaj Kodilkar Reviewed-by: Vasant Hegde Message-Id: <20250801060507.3382-3-sarunkod@amd.com> Message-ID: <948a6ac3-ded9-475b-8c45-9d36220b442b@linaro.org> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 5 ++--- hw/i386/amd_iommu.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 7fb0bb68f0..037e78056d 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -646,7 +646,6 @@ static void amdvi_handle_control_write(AMDVIState *s) unsigned long control = amdvi_readq(s, AMDVI_MMIO_CONTROL); s->enabled = !!(control & AMDVI_MMIO_CONTROL_AMDVIEN); - s->ats_enabled = !!(control & AMDVI_MMIO_CONTROL_HTTUNEN); s->evtlog_enabled = s->enabled && !!(control & AMDVI_MMIO_CONTROL_EVENTLOGEN); @@ -1555,7 +1554,6 @@ static void amdvi_init(AMDVIState *s) s->excl_allow = false; s->mmio_enabled = false; s->enabled = false; - s->ats_enabled = false; s->cmdbuf_enabled = false; /* reset MMIO */ @@ -1626,7 +1624,8 @@ static const VMStateDescription vmstate_amdvi_sysbus_migratable = { /* Updated in amdvi_handle_control_write() */ VMSTATE_BOOL(enabled, AMDVIState), VMSTATE_BOOL(ga_enabled, AMDVIState), - VMSTATE_BOOL(ats_enabled, AMDVIState), + /* bool ats_enabled is obsolete */ + VMSTATE_UNUSED(1), /* was ats_enabled */ VMSTATE_BOOL(cmdbuf_enabled, AMDVIState), VMSTATE_BOOL(completion_wait_intr, AMDVIState), VMSTATE_BOOL(evtlog_enabled, AMDVIState), diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h index 8b42913ed8..67078c6f1e 100644 --- a/hw/i386/amd_iommu.h +++ b/hw/i386/amd_iommu.h @@ -322,7 +322,6 @@ struct AMDVIState { uint64_t mmio_addr; bool enabled; /* IOMMU enabled */ - bool ats_enabled; /* address translation enabled */ bool cmdbuf_enabled; /* command buffer enabled */ bool evtlog_enabled; /* event log enabled */ bool excl_enabled; From a7842d94067cddc80b47ac42fb6e49e2fc02a3c5 Mon Sep 17 00:00:00 2001 From: Sairaj Kodilkar Date: Fri, 1 Aug 2025 11:35:04 +0530 Subject: [PATCH 08/17] hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end Setting up IOAPIC memory region requires mr_sys and mr_ir. Currently these two memory regions are setup after the initializing the IOAPIC memory region, which cause `amdvi_host_dma_iommu()` to use unitialized mr_sys and mr_ir. Move the IOAPIC memory region initialization to the end in order to use the mr_sys and mr_ir regions after they are fully initialized. Fixes: 577c470f4326 ("x86_iommu/amd: Prepare for interrupt remap support") Signed-off-by: Sairaj Kodilkar Reviewed-by: Vasant Hegde Message-Id: <20250801060507.3382-4-sarunkod@amd.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 037e78056d..7308611bf1 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1698,9 +1698,6 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) s->iotlb = g_hash_table_new_full(amdvi_uint64_hash, amdvi_uint64_equal, g_free, g_free); - /* Pseudo address space under root PCI bus. */ - x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID); - /* set up MMIO */ memory_region_init_io(&s->mr_mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio", AMDVI_MMIO_SIZE); @@ -1723,6 +1720,9 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST, &s->mr_ir, 1); + /* Pseudo address space under root PCI bus. */ + x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID); + if (kvm_enabled() && x86ms->apic_id_limit > 255 && !s->xtsup) { error_report("AMD IOMMU with x2APIC configuration requires xtsup=on"); exit(EXIT_FAILURE); From 47d3b32d6fb1c6ec8afb78d12d2420dbbb4c3499 Mon Sep 17 00:00:00 2001 From: Sairaj Kodilkar Date: Fri, 1 Aug 2025 11:35:05 +0530 Subject: [PATCH 09/17] hw/i386/amd_iommu: Fix amdvi_write*() amdvi_write*() function do not preserve the older values of W1C bits in the MMIO register. This results in all W1C bits set to 0, when guest tries to reset a single bit by writing 1 to it. Fix this by preserving W1C bits in the old value of the MMIO register. Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU") Suggested-by: Ethan MILON Signed-off-by: Sairaj Kodilkar Message-Id: <20250801060507.3382-5-sarunkod@amd.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 7308611bf1..c9c32cf7b0 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -123,8 +123,13 @@ static void amdvi_writew(AMDVIState *s, hwaddr addr, uint16_t val) uint16_t romask = lduw_le_p(&s->romask[addr]); uint16_t w1cmask = lduw_le_p(&s->w1cmask[addr]); uint16_t oldval = lduw_le_p(&s->mmior[addr]); + + uint16_t oldval_preserved = oldval & (romask | w1cmask); + uint16_t newval_write = val & ~romask; + uint16_t newval_w1c_set = val & w1cmask; + stw_le_p(&s->mmior[addr], - ((oldval & romask) | (val & ~romask)) & ~(val & w1cmask)); + (oldval_preserved | newval_write) & ~newval_w1c_set); } static void amdvi_writel(AMDVIState *s, hwaddr addr, uint32_t val) @@ -132,8 +137,13 @@ static void amdvi_writel(AMDVIState *s, hwaddr addr, uint32_t val) uint32_t romask = ldl_le_p(&s->romask[addr]); uint32_t w1cmask = ldl_le_p(&s->w1cmask[addr]); uint32_t oldval = ldl_le_p(&s->mmior[addr]); + + uint32_t oldval_preserved = oldval & (romask | w1cmask); + uint32_t newval_write = val & ~romask; + uint32_t newval_w1c_set = val & w1cmask; + stl_le_p(&s->mmior[addr], - ((oldval & romask) | (val & ~romask)) & ~(val & w1cmask)); + (oldval_preserved | newval_write) & ~newval_w1c_set); } static void amdvi_writeq(AMDVIState *s, hwaddr addr, uint64_t val) @@ -141,8 +151,13 @@ static void amdvi_writeq(AMDVIState *s, hwaddr addr, uint64_t val) uint64_t romask = ldq_le_p(&s->romask[addr]); uint64_t w1cmask = ldq_le_p(&s->w1cmask[addr]); uint64_t oldval = ldq_le_p(&s->mmior[addr]); + + uint64_t oldval_preserved = oldval & (romask | w1cmask); + uint64_t newval_write = val & ~romask; + uint64_t newval_w1c_set = val & w1cmask; + stq_le_p(&s->mmior[addr], - ((oldval & romask) | (val & ~romask)) & ~(val & w1cmask)); + (oldval_preserved | newval_write) & ~newval_w1c_set); } /* OR a 64-bit register with a 64-bit value */ From 10690920b0efb3ed8b166443bae8077104bb129d Mon Sep 17 00:00:00 2001 From: Sairaj Kodilkar Date: Fri, 1 Aug 2025 11:35:06 +0530 Subject: [PATCH 10/17] hw/i386/amd_iommu: Support MMIO writes to the status register Support the writes to the status register so that guest can reset the EventOverflow, EventLogInt, ComWaitIntr, etc bits after servicing the respective interrupt. Signed-off-by: Sairaj Kodilkar Reviewed-by: Vasant Hegde Message-Id: <20250801060507.3382-6-sarunkod@amd.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index c9c32cf7b0..6925085d29 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -862,6 +862,9 @@ static void amdvi_mmio_write(void *opaque, hwaddr addr, uint64_t val, amdvi_mmio_reg_write(s, size, val, addr); amdvi_handle_pprtail_write(s); break; + case AMDVI_MMIO_STATUS: + amdvi_mmio_reg_write(s, size, val, addr); + break; } } From c0ef803a879b97f3d269348c968fb3874c2761f6 Mon Sep 17 00:00:00 2001 From: Sairaj Kodilkar Date: Fri, 1 Aug 2025 11:35:07 +0530 Subject: [PATCH 11/17] hw/i386/amd_iommu: Fix event log generation Current event logging code is broken, because of following issues 1. The code uses '|' instead of '&' to test the bit field, which causes vIOMMU to generate overflow interrupt for every log entry. 2. Code does not update the eventlog tail MMIO register after adding an entry to the buffer, because of which guest cannot process new entries (as head == tail means buffer is empty). 3. Compares eventlog tail (which is byte offset in the buffer) to eventlog length (which is number of maximum entries in the buffer). This causes vIOMMU to generate only fix number of event logs, after which it keeps on generating overflow interrupts, without actually resetting the log buffer. 4. Updates ComWaitInt instead of EventLogInt bitfield in Status register. Guest checks this field to see if there are new event log entries in the buffer. 5. Does not reset event log head and tail pointers when guest writes to eventlog base register. Fix above issues, so that guest can process event log entries. Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU") Signed-off-by: Sairaj Kodilkar Reviewed-by: Vasant Hegde Message-Id: <20250801060507.3382-7-sarunkod@amd.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 44 +++++++++++++++++++++++++++++++++++--------- hw/i386/amd_iommu.h | 1 + 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 6925085d29..26be69bec8 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -160,10 +160,10 @@ static void amdvi_writeq(AMDVIState *s, hwaddr addr, uint64_t val) (oldval_preserved | newval_write) & ~newval_w1c_set); } -/* OR a 64-bit register with a 64-bit value */ +/* AND a 64-bit register with a 64-bit value */ static bool amdvi_test_mask(AMDVIState *s, hwaddr addr, uint64_t val) { - return amdvi_readq(s, addr) | val; + return amdvi_readq(s, addr) & val; } /* OR a 64-bit register with a 64-bit value storing result in the register */ @@ -192,19 +192,31 @@ static void amdvi_generate_msi_interrupt(AMDVIState *s) } } +static uint32_t get_next_eventlog_entry(AMDVIState *s) +{ + uint32_t evtlog_size = s->evtlog_len * AMDVI_EVENT_LEN; + return (s->evtlog_tail + AMDVI_EVENT_LEN) % evtlog_size; +} + static void amdvi_log_event(AMDVIState *s, uint64_t *evt) { + uint32_t evtlog_tail_next; + /* event logging not enabled */ if (!s->evtlog_enabled || amdvi_test_mask(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_EVT_OVF)) { return; } + evtlog_tail_next = get_next_eventlog_entry(s); + /* event log buffer full */ - if (s->evtlog_tail >= s->evtlog_len) { - amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_EVT_OVF); - /* generate interrupt */ - amdvi_generate_msi_interrupt(s); + if (evtlog_tail_next == s->evtlog_head) { + /* generate overflow interrupt */ + if (s->evtlog_intr) { + amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_EVT_OVF); + amdvi_generate_msi_interrupt(s); + } return; } @@ -213,9 +225,13 @@ static void amdvi_log_event(AMDVIState *s, uint64_t *evt) trace_amdvi_evntlog_fail(s->evtlog, s->evtlog_tail); } - s->evtlog_tail += AMDVI_EVENT_LEN; - amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT); - amdvi_generate_msi_interrupt(s); + s->evtlog_tail = evtlog_tail_next; + amdvi_writeq_raw(s, AMDVI_MMIO_EVENT_TAIL, s->evtlog_tail); + + if (s->evtlog_intr) { + amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_EVENT_INT); + amdvi_generate_msi_interrupt(s); + } } static void amdvi_setevent_bits(uint64_t *buffer, uint64_t value, int start, @@ -731,9 +747,19 @@ static inline void amdvi_handle_excllim_write(AMDVIState *s) static inline void amdvi_handle_evtbase_write(AMDVIState *s) { uint64_t val = amdvi_readq(s, AMDVI_MMIO_EVENT_BASE); + + if (amdvi_readq(s, AMDVI_MMIO_STATUS) & AMDVI_MMIO_STATUS_EVENT_INT) + /* Do not reset if eventlog interrupt bit is set*/ + return; + s->evtlog = val & AMDVI_MMIO_EVTLOG_BASE_MASK; s->evtlog_len = 1UL << (amdvi_readq(s, AMDVI_MMIO_EVTLOG_SIZE_BYTE) & AMDVI_MMIO_EVTLOG_SIZE_MASK); + + /* clear tail and head pointer to 0 when event base is updated */ + s->evtlog_tail = s->evtlog_head = 0; + amdvi_writeq_raw(s, AMDVI_MMIO_EVENT_HEAD, s->evtlog_head); + amdvi_writeq_raw(s, AMDVI_MMIO_EVENT_TAIL, s->evtlog_tail); } static inline void amdvi_handle_evttail_write(AMDVIState *s) diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h index 67078c6f1e..2476296c49 100644 --- a/hw/i386/amd_iommu.h +++ b/hw/i386/amd_iommu.h @@ -111,6 +111,7 @@ #define AMDVI_MMIO_STATUS_CMDBUF_RUN (1 << 4) #define AMDVI_MMIO_STATUS_EVT_RUN (1 << 3) #define AMDVI_MMIO_STATUS_COMP_INT (1 << 2) +#define AMDVI_MMIO_STATUS_EVENT_INT (1 << 1) #define AMDVI_MMIO_STATUS_EVT_OVF (1 << 0) #define AMDVI_CMDBUF_ID_BYTE 0x07 From 8d5613d2eefa6ca94b3c642583488e20915c48f7 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Thu, 12 Jun 2025 17:17:25 +0200 Subject: [PATCH 12/17] tests/acpi: virt: add an empty HEST file Such file will be used to track HEST table changes. For now, disallow HEST table check until we update it to the current data. Signed-off-by: Mauro Carvalho Chehab Acked-by: Igor Mammedov Reviewed-by: Jonathan Cameron Message-Id: --- tests/data/acpi/aarch64/virt/HEST | 0 tests/qtest/bios-tables-test-allowed-diff.h | 1 + 2 files changed, 1 insertion(+) create mode 100644 tests/data/acpi/aarch64/virt/HEST diff --git a/tests/data/acpi/aarch64/virt/HEST b/tests/data/acpi/aarch64/virt/HEST new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..39901c58d6 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/aarch64/virt/HEST", From 5088651138b94542807414eb841363b16d6aa535 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Thu, 12 Jun 2025 17:17:26 +0200 Subject: [PATCH 13/17] tests/qtest/bios-tables-test: extend to also check HEST table Currently, aarch64 can generate a HEST table when loaded with -machine ras=on. Add support for it. Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Igor Mammedov Reviewed-by: Jonathan Cameron Message-Id: <9ce77140500ef68cc939d63952c25579f711ea52.1749741085.git.mchehab+huawei@kernel.org> --- tests/qtest/bios-tables-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 6aec68decc..e7e6926c81 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -2208,7 +2208,7 @@ static void test_acpi_aarch64_virt_tcg(void) data.smbios_cpu_max_speed = 2900; data.smbios_cpu_curr_speed = 2700; - test_acpi_one("-cpu cortex-a57 " + test_acpi_one("-cpu cortex-a57 -machine ras=on " "-smbios type=4,max-speed=2900,current-speed=2700", &data); free_test_data(&data); } From cd16f08ad4bda8191e4de986d17184c9da5466cd Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Thu, 12 Jun 2025 17:17:27 +0200 Subject: [PATCH 14/17] tests/acpi: virt: update HEST file with its current data Now that HEST table is checked for aarch64, add the current firmware file. Signed-off-by: Mauro Carvalho Chehab Acked-by: Igor Mammedov Reviewed-by: Jonathan Cameron Message-Id: --- tests/data/acpi/aarch64/virt/HEST | Bin 0 -> 132 bytes tests/qtest/bios-tables-test-allowed-diff.h | 1 - 2 files changed, 1 deletion(-) diff --git a/tests/data/acpi/aarch64/virt/HEST b/tests/data/acpi/aarch64/virt/HEST index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..4c5d8c5b5da5b3241f93cd0839e94272bf6b1486 100644 GIT binary patch literal 132 zcmeZp4Gw8xU|?W; Date: Mon, 14 Jul 2025 09:00:47 +0100 Subject: [PATCH 15/17] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait FreeBSD does both, and this appears to be perfectly valid. The VT-d spec even talks about the ordering (the status write should be done first, unsurprisingly). We certainly shouldn't assert() and abort QEMU if the guest asks for both. Fixes: ed7b8fbcfb88 ("intel-iommu: add supports for queued invalidation interface") Closes: https://gitlab.com/qemu-project/qemu/-/issues/3028 Signed-off-by: David Woodhouse Message-Id: <0122cbabc0adcc3cf878f5fd7834d8f258c7a2f2.camel@infradead.org> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index fe9a5f2872..83c5e44413 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2828,6 +2828,7 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc) { uint64_t mask[4] = {VTD_INV_DESC_WAIT_RSVD_LO, VTD_INV_DESC_WAIT_RSVD_HI, VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE}; + bool ret = true; if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false, __func__, "wait")) { @@ -2839,8 +2840,6 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc) uint32_t status_data = (uint32_t)(inv_desc->lo >> VTD_INV_DESC_WAIT_DATA_SHIFT); - assert(!(inv_desc->lo & VTD_INV_DESC_WAIT_IF)); - /* FIXME: need to be masked with HAW? */ dma_addr_t status_addr = inv_desc->hi; trace_vtd_inv_desc_wait_sw(status_addr, status_data); @@ -2849,18 +2848,22 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc) &status_data, sizeof(status_data), MEMTXATTRS_UNSPECIFIED)) { trace_vtd_inv_desc_wait_write_fail(inv_desc->hi, inv_desc->lo); - return false; + ret = false; } - } else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) { + } + + if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) { /* Interrupt flag */ vtd_generate_completion_event(s); - } else { + } + + if (!(inv_desc->lo & (VTD_INV_DESC_WAIT_IF | VTD_INV_DESC_WAIT_SW))) { error_report_once("%s: invalid wait desc: hi=%"PRIx64", lo=%"PRIx64 " (unknown type)", __func__, inv_desc->hi, inv_desc->lo); return false; } - return true; + return ret; } static bool vtd_process_context_cache_desc(IntelIOMMUState *s, From 4164adc476d85d46ef4901c05a9807b24473b00d Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Mon, 14 Jul 2025 12:26:23 +0200 Subject: [PATCH 16/17] MAINTAINERS: add net/vhost* files under `vhost` net/vhost* files should be interesting for vhost maintainers/reviewers. Suggested-by: Peter Maydell Signed-off-by: Stefano Garzarella Message-Id: <20250714102626.34431-1-sgarzare@redhat.com> Reviewed-by: Manos Pitsidianakis Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 069d77f2f8..28cea34271 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2322,6 +2322,7 @@ F: include/*/vhost* F: subprojects/libvhost-user/ F: block/export/vhost-user* F: util/vhost-user-server.c +F: net/vhost* vhost-shadow-virtqueue R: Eugenio Pérez From 4caf74916d09019e61c91f8cb1166510836d35e8 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Mon, 14 Jul 2025 12:11:56 +0200 Subject: [PATCH 17/17] net/vdpa: fix potential fd leak in net_init_vhost_vdpa() Coverity reported a file descriptor leak (CID 1490785) that happens if `vhost_vdpa_get_max_queue_pairs()` returns 0, since in that case net_host_vdpa_init(), which should take ownership of the fd, is never called. vhost_vdpa_get_max_queue_pairs() returns 1 if VIRTIO_NET_F_MQ is not negotiated, or a negative error if the ioctl() fails, or the maximum number of queue pairs exposed by the device in the config space in the `max_virtqueue_pairs` field. In the VIRTIO spec we have: The device MUST set max_virtqueue_pairs to between 1 and 0x8000 inclusive, if it offers VIRTIO_NET_F_MQ. So, if `vhost_vdpa_get_max_queue_pairs()` returns 0, it's really an error since the device is violating the VIRTIO spec. Treat also `queue_pairs == 0` as an error, and jump to the `err` label, to return a negative value to the caller in any case. Coverity: CID 1490785 Suggested-by: Peter Maydell Signed-off-by: Stefano Garzarella Message-Id: <20250714101156.30024-1-sgarzare@redhat.com> Suggested-by: Peter Maydell Signed-off-by: Stefano Garzarella Reviewed-by: Manos Pitsidianakis Acked-by: Jason Wang --- net/vhost-vdpa.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 6a30a44d2b..74d26a9497 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -1840,9 +1840,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name, queue_pairs = vhost_vdpa_get_max_queue_pairs(vdpa_device_fd, features, &has_cvq, errp); - if (queue_pairs < 0) { - qemu_close(vdpa_device_fd); - return queue_pairs; + if (queue_pairs <= 0) { + goto err; } r = vhost_vdpa_get_iova_range(vdpa_device_fd, &iova_range);