From 81ee17aa461d1bf05a45dddd7f9abfa9ab0b2036 Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 25 Nov 2020 01:30:55 +0000 Subject: [PATCH 01/65] vhost-user-scsi: Fix memleaks in vus_proc_req() The 'elem' is allocated memory in vu_queue_pop(), and its memory should be freed in all error branches after vu_queue_pop(). In addition, in order to free the 'elem' memory outside of while(1) loop, move the definition of 'elem' to the beginning of vus_proc_req(). Reported-by: Euler Robot Signed-off-by: Alex Chen Reviewed-by: Raphael Norwitz Message-Id: <20201125013055.34147-1-alex.chen@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-scsi/vhost-user-scsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c index 0f9ba4b2a2..4639440a70 100644 --- a/contrib/vhost-user-scsi/vhost-user-scsi.c +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c @@ -232,6 +232,7 @@ static void vus_proc_req(VuDev *vu_dev, int idx) VugDev *gdev; VusDev *vdev_scsi; VuVirtq *vq; + VuVirtqElement *elem = NULL; assert(vu_dev); @@ -248,7 +249,6 @@ static void vus_proc_req(VuDev *vu_dev, int idx) g_debug("Got kicked on vq[%d]@%p", idx, vq); while (1) { - VuVirtqElement *elem; VirtIOSCSICmdReq *req; VirtIOSCSICmdResp *rsp; @@ -288,6 +288,7 @@ static void vus_proc_req(VuDev *vu_dev, int idx) free(elem); } + free(elem); } static void vus_queue_set_started(VuDev *vu_dev, int idx, bool started) From 3b5ebf8532afdc1518bd8b0961ed802bc3f5f07c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eugenio=20P=C3=A9rez?= Date: Mon, 16 Nov 2020 17:55:02 +0100 Subject: [PATCH 02/65] memory: Rename memory_region_notify_one to memory_region_notify_iommu_one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous name didn't reflect the iommu operation. Signed-off-by: Eugenio Pérez Reviewed-by: Peter Xu Reviewed-by: David Gibson Reviewed-by: Juan Quintela Reviewed-by: Eric Auger Acked-by: Jason Wang Message-Id: <20201116165506.31315-2-eperezma@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/smmu-common.c | 2 +- hw/arm/smmuv3.c | 2 +- hw/i386/intel_iommu.c | 4 ++-- include/exec/memory.h | 6 +++--- softmmu/memory.c | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 3838db1395..88d2c454f0 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -472,7 +472,7 @@ static void smmu_unmap_notifier_range(IOMMUNotifier *n) entry.perm = IOMMU_NONE; entry.addr_mask = n->end - n->start; - memory_region_notify_one(n, &entry); + memory_region_notify_iommu_one(n, &entry); } /* Unmap all notifiers attached to @mr */ diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 22607c3784..273f5f7dce 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -828,7 +828,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr, entry.addr_mask = num_pages * (1 << granule) - 1; entry.perm = IOMMU_NONE; - memory_region_notify_one(n, &entry); + memory_region_notify_iommu_one(n, &entry); } /* invalidate an asid/iova range tuple in all mr's */ diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 70ac837733..067593b9e4 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3497,7 +3497,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) /* This field is meaningless for unmap */ entry.translated_addr = 0; - memory_region_notify_one(n, &entry); + memory_region_notify_iommu_one(n, &entry); start += mask; remain -= mask; @@ -3535,7 +3535,7 @@ static void vtd_address_space_refresh_all(IntelIOMMUState *s) static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private) { - memory_region_notify_one((IOMMUNotifier *)private, entry); + memory_region_notify_iommu_one((IOMMUNotifier *)private, entry); return 0; } diff --git a/include/exec/memory.h b/include/exec/memory.h index 0f3e6bcd5e..d8456ccf52 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -236,7 +236,7 @@ enum IOMMUMemoryRegionAttr { * The IOMMU implementation must use the IOMMU notifier infrastructure * to report whenever mappings are changed, by calling * memory_region_notify_iommu() (or, if necessary, by calling - * memory_region_notify_one() for each registered notifier). + * memory_region_notify_iommu_one() for each registered notifier). * * Conceptually an IOMMU provides a mapping from input address * to an output TLB entry. If the IOMMU is aware of memory transaction @@ -1346,7 +1346,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, IOMMUTLBEntry entry); /** - * memory_region_notify_one: notify a change in an IOMMU translation + * memory_region_notify_iommu_one: notify a change in an IOMMU translation * entry to a single notifier * * This works just like memory_region_notify_iommu(), but it only @@ -1357,7 +1357,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, * replaces all old entries for the same virtual I/O address range. * Deleted entries have .@perm == 0. */ -void memory_region_notify_one(IOMMUNotifier *notifier, +void memory_region_notify_iommu_one(IOMMUNotifier *notifier, IOMMUTLBEntry *entry); /** diff --git a/softmmu/memory.c b/softmmu/memory.c index 11ca94d037..44de610c72 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1942,8 +1942,8 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, memory_region_update_iommu_notify_flags(iommu_mr, NULL); } -void memory_region_notify_one(IOMMUNotifier *notifier, - IOMMUTLBEntry *entry) +void memory_region_notify_iommu_one(IOMMUNotifier *notifier, + IOMMUTLBEntry *entry) { IOMMUNotifierFlag request_flags; hwaddr entry_end = entry->iova + entry->addr_mask; @@ -1979,7 +1979,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, IOMMU_NOTIFIER_FOREACH(iommu_notifier, iommu_mr) { if (iommu_notifier->iommu_idx == iommu_idx) { - memory_region_notify_one(iommu_notifier, &entry); + memory_region_notify_iommu_one(iommu_notifier, &entry); } } } From 5039caf3c449c49e625d34e134463260cf8e00e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eugenio=20P=C3=A9rez?= Date: Mon, 16 Nov 2020 17:55:03 +0100 Subject: [PATCH 03/65] memory: Add IOMMUTLBEvent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This way we can tell between regular IOMMUTLBEntry (entry of IOMMU hardware) and notifications. In the notifications, we set explicitly if it is a MAPs or an UNMAP, instead of trusting in entry permissions to differentiate them. Signed-off-by: Eugenio Pérez Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Acked-by: Jason Wang Message-Id: <20201116165506.31315-3-eperezma@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Matthew Rosato Acked-by: David Gibson --- hw/arm/smmu-common.c | 13 +++--- hw/arm/smmuv3.c | 13 +++--- hw/i386/intel_iommu.c | 88 ++++++++++++++++++++++------------------ hw/misc/tz-mpc.c | 32 ++++++++------- hw/ppc/spapr_iommu.c | 15 +++---- hw/s390x/s390-pci-inst.c | 27 +++++++----- hw/virtio/virtio-iommu.c | 30 +++++++------- include/exec/memory.h | 27 ++++++------ softmmu/memory.c | 20 ++++----- 9 files changed, 143 insertions(+), 122 deletions(-) diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 88d2c454f0..405d5c5325 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -465,14 +465,15 @@ IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid) /* Unmap the whole notifier's range */ static void smmu_unmap_notifier_range(IOMMUNotifier *n) { - IOMMUTLBEntry entry; + IOMMUTLBEvent event; - entry.target_as = &address_space_memory; - entry.iova = n->start; - entry.perm = IOMMU_NONE; - entry.addr_mask = n->end - n->start; + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.target_as = &address_space_memory; + event.entry.iova = n->start; + event.entry.perm = IOMMU_NONE; + event.entry.addr_mask = n->end - n->start; - memory_region_notify_iommu_one(n, &entry); + memory_region_notify_iommu_one(n, &event); } /* Unmap all notifiers attached to @mr */ diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 273f5f7dce..bbca0e9f20 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -800,7 +800,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr, uint8_t tg, uint64_t num_pages) { SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu); - IOMMUTLBEntry entry; + IOMMUTLBEvent event; uint8_t granule = tg; if (!tg) { @@ -823,12 +823,13 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr, granule = tt->granule_sz; } - entry.target_as = &address_space_memory; - entry.iova = iova; - entry.addr_mask = num_pages * (1 << granule) - 1; - entry.perm = IOMMU_NONE; + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.target_as = &address_space_memory; + event.entry.iova = iova; + event.entry.addr_mask = num_pages * (1 << granule) - 1; + event.entry.perm = IOMMU_NONE; - memory_region_notify_iommu_one(n, &entry); + memory_region_notify_iommu_one(n, &event); } /* invalidate an asid/iova range tuple in all mr's */ diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 067593b9e4..56180b1c43 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -1073,7 +1073,7 @@ static int vtd_iova_to_slpte(IntelIOMMUState *s, VTDContextEntry *ce, } } -typedef int (*vtd_page_walk_hook)(IOMMUTLBEntry *entry, void *private); +typedef int (*vtd_page_walk_hook)(IOMMUTLBEvent *event, void *private); /** * Constant information used during page walking @@ -1094,11 +1094,12 @@ typedef struct { uint16_t domain_id; } vtd_page_walk_info; -static int vtd_page_walk_one(IOMMUTLBEntry *entry, vtd_page_walk_info *info) +static int vtd_page_walk_one(IOMMUTLBEvent *event, vtd_page_walk_info *info) { VTDAddressSpace *as = info->as; vtd_page_walk_hook hook_fn = info->hook_fn; void *private = info->private; + IOMMUTLBEntry *entry = &event->entry; DMAMap target = { .iova = entry->iova, .size = entry->addr_mask, @@ -1107,7 +1108,7 @@ static int vtd_page_walk_one(IOMMUTLBEntry *entry, vtd_page_walk_info *info) }; DMAMap *mapped = iova_tree_find(as->iova_tree, &target); - if (entry->perm == IOMMU_NONE && !info->notify_unmap) { + if (event->type == IOMMU_NOTIFIER_UNMAP && !info->notify_unmap) { trace_vtd_page_walk_one_skip_unmap(entry->iova, entry->addr_mask); return 0; } @@ -1115,7 +1116,7 @@ static int vtd_page_walk_one(IOMMUTLBEntry *entry, vtd_page_walk_info *info) assert(hook_fn); /* Update local IOVA mapped ranges */ - if (entry->perm) { + if (event->type == IOMMU_NOTIFIER_MAP) { if (mapped) { /* If it's exactly the same translation, skip */ if (!memcmp(mapped, &target, sizeof(target))) { @@ -1141,19 +1142,21 @@ static int vtd_page_walk_one(IOMMUTLBEntry *entry, vtd_page_walk_info *info) int ret; /* Emulate an UNMAP */ + event->type = IOMMU_NOTIFIER_UNMAP; entry->perm = IOMMU_NONE; trace_vtd_page_walk_one(info->domain_id, entry->iova, entry->translated_addr, entry->addr_mask, entry->perm); - ret = hook_fn(entry, private); + ret = hook_fn(event, private); if (ret) { return ret; } /* Drop any existing mapping */ iova_tree_remove(as->iova_tree, &target); - /* Recover the correct permission */ + /* Recover the correct type */ + event->type = IOMMU_NOTIFIER_MAP; entry->perm = cache_perm; } } @@ -1170,7 +1173,7 @@ static int vtd_page_walk_one(IOMMUTLBEntry *entry, vtd_page_walk_info *info) trace_vtd_page_walk_one(info->domain_id, entry->iova, entry->translated_addr, entry->addr_mask, entry->perm); - return hook_fn(entry, private); + return hook_fn(event, private); } /** @@ -1191,7 +1194,7 @@ static int vtd_page_walk_level(dma_addr_t addr, uint64_t start, uint32_t offset; uint64_t slpte; uint64_t subpage_size, subpage_mask; - IOMMUTLBEntry entry; + IOMMUTLBEvent event; uint64_t iova = start; uint64_t iova_next; int ret = 0; @@ -1245,13 +1248,15 @@ static int vtd_page_walk_level(dma_addr_t addr, uint64_t start, * * In either case, we send an IOTLB notification down. */ - entry.target_as = &address_space_memory; - entry.iova = iova & subpage_mask; - entry.perm = IOMMU_ACCESS_FLAG(read_cur, write_cur); - entry.addr_mask = ~subpage_mask; + event.entry.target_as = &address_space_memory; + event.entry.iova = iova & subpage_mask; + event.entry.perm = IOMMU_ACCESS_FLAG(read_cur, write_cur); + event.entry.addr_mask = ~subpage_mask; /* NOTE: this is only meaningful if entry_valid == true */ - entry.translated_addr = vtd_get_slpte_addr(slpte, info->aw); - ret = vtd_page_walk_one(&entry, info); + event.entry.translated_addr = vtd_get_slpte_addr(slpte, info->aw); + event.type = event.entry.perm ? IOMMU_NOTIFIER_MAP : + IOMMU_NOTIFIER_UNMAP; + ret = vtd_page_walk_one(&event, info); } if (ret < 0) { @@ -1430,10 +1435,10 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, return 0; } -static int vtd_sync_shadow_page_hook(IOMMUTLBEntry *entry, +static int vtd_sync_shadow_page_hook(IOMMUTLBEvent *event, void *private) { - memory_region_notify_iommu((IOMMUMemoryRegion *)private, 0, *entry); + memory_region_notify_iommu(private, 0, *event); return 0; } @@ -1993,14 +1998,17 @@ static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s, * page tables. We just deliver the PSI down to * invalidate caches. */ - IOMMUTLBEntry entry = { - .target_as = &address_space_memory, - .iova = addr, - .translated_addr = 0, - .addr_mask = size - 1, - .perm = IOMMU_NONE, + IOMMUTLBEvent event = { + .type = IOMMU_NOTIFIER_UNMAP, + .entry = { + .target_as = &address_space_memory, + .iova = addr, + .translated_addr = 0, + .addr_mask = size - 1, + .perm = IOMMU_NONE, + }, }; - memory_region_notify_iommu(&vtd_as->iommu, 0, entry); + memory_region_notify_iommu(&vtd_as->iommu, 0, event); } } } @@ -2412,7 +2420,7 @@ static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc) { VTDAddressSpace *vtd_dev_as; - IOMMUTLBEntry entry; + IOMMUTLBEvent event; struct VTDBus *vtd_bus; hwaddr addr; uint64_t sz; @@ -2460,12 +2468,13 @@ static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s, sz = VTD_PAGE_SIZE; } - entry.target_as = &vtd_dev_as->as; - entry.addr_mask = sz - 1; - entry.iova = addr; - entry.perm = IOMMU_NONE; - entry.translated_addr = 0; - memory_region_notify_iommu(&vtd_dev_as->iommu, 0, entry); + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.target_as = &vtd_dev_as->as; + event.entry.addr_mask = sz - 1; + event.entry.iova = addr; + event.entry.perm = IOMMU_NONE; + event.entry.translated_addr = 0; + memory_region_notify_iommu(&vtd_dev_as->iommu, 0, event); done: return true; @@ -3485,19 +3494,20 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) size = remain = end - start + 1; while (remain >= VTD_PAGE_SIZE) { - IOMMUTLBEntry entry; + IOMMUTLBEvent event; uint64_t mask = get_naturally_aligned_size(start, remain, s->aw_bits); assert(mask); - entry.iova = start; - entry.addr_mask = mask - 1; - entry.target_as = &address_space_memory; - entry.perm = IOMMU_NONE; + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.iova = start; + event.entry.addr_mask = mask - 1; + event.entry.target_as = &address_space_memory; + event.entry.perm = IOMMU_NONE; /* This field is meaningless for unmap */ - entry.translated_addr = 0; + event.entry.translated_addr = 0; - memory_region_notify_iommu_one(n, &entry); + memory_region_notify_iommu_one(n, &event); start += mask; remain -= mask; @@ -3533,9 +3543,9 @@ static void vtd_address_space_refresh_all(IntelIOMMUState *s) vtd_switch_address_space_all(s); } -static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private) +static int vtd_replay_hook(IOMMUTLBEvent *event, void *private) { - memory_region_notify_iommu_one((IOMMUNotifier *)private, entry); + memory_region_notify_iommu_one(private, event); return 0; } diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c index 98f151237f..30481e1c90 100644 --- a/hw/misc/tz-mpc.c +++ b/hw/misc/tz-mpc.c @@ -82,8 +82,10 @@ static void tz_mpc_iommu_notify(TZMPC *s, uint32_t lutidx, /* Called when the LUT word at lutidx has changed from oldlut to newlut; * must call the IOMMU notifiers for the changed blocks. */ - IOMMUTLBEntry entry = { - .addr_mask = s->blocksize - 1, + IOMMUTLBEvent event = { + .entry = { + .addr_mask = s->blocksize - 1, + } }; hwaddr addr = lutidx * s->blocksize * 32; int i; @@ -100,26 +102,28 @@ static void tz_mpc_iommu_notify(TZMPC *s, uint32_t lutidx, block_is_ns = newlut & (1 << i); trace_tz_mpc_iommu_notify(addr); - entry.iova = addr; - entry.translated_addr = addr; + event.entry.iova = addr; + event.entry.translated_addr = addr; - entry.perm = IOMMU_NONE; - memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, entry); - memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, entry); + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.perm = IOMMU_NONE; + memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, event); + memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, event); - entry.perm = IOMMU_RW; + event.type = IOMMU_NOTIFIER_MAP; + event.entry.perm = IOMMU_RW; if (block_is_ns) { - entry.target_as = &s->blocked_io_as; + event.entry.target_as = &s->blocked_io_as; } else { - entry.target_as = &s->downstream_as; + event.entry.target_as = &s->downstream_as; } - memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, entry); + memory_region_notify_iommu(&s->upstream, IOMMU_IDX_S, event); if (block_is_ns) { - entry.target_as = &s->downstream_as; + event.entry.target_as = &s->downstream_as; } else { - entry.target_as = &s->blocked_io_as; + event.entry.target_as = &s->blocked_io_as; } - memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, entry); + memory_region_notify_iommu(&s->upstream, IOMMU_IDX_NS, event); } } diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index 0790239ba5..30352df00e 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -445,7 +445,7 @@ static void spapr_tce_reset(DeviceState *dev) static target_ulong put_tce_emu(SpaprTceTable *tcet, target_ulong ioba, target_ulong tce) { - IOMMUTLBEntry entry; + IOMMUTLBEvent event; hwaddr page_mask = IOMMU_PAGE_MASK(tcet->page_shift); unsigned long index = (ioba - tcet->bus_offset) >> tcet->page_shift; @@ -457,12 +457,13 @@ static target_ulong put_tce_emu(SpaprTceTable *tcet, target_ulong ioba, tcet->table[index] = tce; - entry.target_as = &address_space_memory, - entry.iova = (ioba - tcet->bus_offset) & page_mask; - entry.translated_addr = tce & page_mask; - entry.addr_mask = ~page_mask; - entry.perm = spapr_tce_iommu_access_flags(tce); - memory_region_notify_iommu(&tcet->iommu, 0, entry); + event.entry.target_as = &address_space_memory, + event.entry.iova = (ioba - tcet->bus_offset) & page_mask; + event.entry.translated_addr = tce & page_mask; + event.entry.addr_mask = ~page_mask; + event.entry.perm = spapr_tce_iommu_access_flags(tce); + event.type = event.entry.perm ? IOMMU_NOTIFIER_MAP : IOMMU_NOTIFIER_UNMAP; + memory_region_notify_iommu(&tcet->iommu, 0, event); return H_SUCCESS; } diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index 70bfd91bf7..d9e1e29f1e 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -602,15 +602,18 @@ static uint32_t s390_pci_update_iotlb(S390PCIIOMMU *iommu, S390IOTLBEntry *entry) { S390IOTLBEntry *cache = g_hash_table_lookup(iommu->iotlb, &entry->iova); - IOMMUTLBEntry notify = { - .target_as = &address_space_memory, - .iova = entry->iova, - .translated_addr = entry->translated_addr, - .perm = entry->perm, - .addr_mask = ~PAGE_MASK, + IOMMUTLBEvent event = { + .type = entry->perm ? IOMMU_NOTIFIER_MAP : IOMMU_NOTIFIER_UNMAP, + .entry = { + .target_as = &address_space_memory, + .iova = entry->iova, + .translated_addr = entry->translated_addr, + .perm = entry->perm, + .addr_mask = ~PAGE_MASK, + }, }; - if (entry->perm == IOMMU_NONE) { + if (event.type == IOMMU_NOTIFIER_UNMAP) { if (!cache) { goto out; } @@ -623,9 +626,11 @@ static uint32_t s390_pci_update_iotlb(S390PCIIOMMU *iommu, goto out; } - notify.perm = IOMMU_NONE; - memory_region_notify_iommu(&iommu->iommu_mr, 0, notify); - notify.perm = entry->perm; + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.perm = IOMMU_NONE; + memory_region_notify_iommu(&iommu->iommu_mr, 0, event); + event.type = IOMMU_NOTIFIER_MAP; + event.entry.perm = entry->perm; } cache = g_new(S390IOTLBEntry, 1); @@ -637,7 +642,7 @@ static uint32_t s390_pci_update_iotlb(S390PCIIOMMU *iommu, dec_dma_avail(iommu); } - memory_region_notify_iommu(&iommu->iommu_mr, 0, notify); + memory_region_notify_iommu(&iommu->iommu_mr, 0, event); out: return iommu->dma_limit ? iommu->dma_limit->avail : 1; diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index fc5c75d693..cea8811295 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -129,7 +129,7 @@ static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr virt_start, hwaddr virt_end, hwaddr paddr, uint32_t flags) { - IOMMUTLBEntry entry; + IOMMUTLBEvent event; IOMMUAccessFlags perm = IOMMU_ACCESS_FLAG(flags & VIRTIO_IOMMU_MAP_F_READ, flags & VIRTIO_IOMMU_MAP_F_WRITE); @@ -141,19 +141,20 @@ static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr virt_start, trace_virtio_iommu_notify_map(mr->parent_obj.name, virt_start, virt_end, paddr, perm); - entry.target_as = &address_space_memory; - entry.addr_mask = virt_end - virt_start; - entry.iova = virt_start; - entry.perm = perm; - entry.translated_addr = paddr; + event.type = IOMMU_NOTIFIER_MAP; + event.entry.target_as = &address_space_memory; + event.entry.addr_mask = virt_end - virt_start; + event.entry.iova = virt_start; + event.entry.perm = perm; + event.entry.translated_addr = paddr; - memory_region_notify_iommu(mr, 0, entry); + memory_region_notify_iommu(mr, 0, event); } static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr virt_start, hwaddr virt_end) { - IOMMUTLBEntry entry; + IOMMUTLBEvent event; if (!(mr->iommu_notify_flags & IOMMU_NOTIFIER_UNMAP)) { return; @@ -161,13 +162,14 @@ static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr virt_start, trace_virtio_iommu_notify_unmap(mr->parent_obj.name, virt_start, virt_end); - entry.target_as = &address_space_memory; - entry.addr_mask = virt_end - virt_start; - entry.iova = virt_start; - entry.perm = IOMMU_NONE; - entry.translated_addr = 0; + event.type = IOMMU_NOTIFIER_UNMAP; + event.entry.target_as = &address_space_memory; + event.entry.addr_mask = virt_end - virt_start; + event.entry.iova = virt_start; + event.entry.perm = IOMMU_NONE; + event.entry.translated_addr = 0; - memory_region_notify_iommu(mr, 0, entry); + memory_region_notify_iommu(mr, 0, event); } static gboolean virtio_iommu_notify_unmap_cb(gpointer key, gpointer value, diff --git a/include/exec/memory.h b/include/exec/memory.h index d8456ccf52..e86b5e92da 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -116,6 +116,11 @@ struct IOMMUNotifier { }; typedef struct IOMMUNotifier IOMMUNotifier; +typedef struct IOMMUTLBEvent { + IOMMUNotifierFlag type; + IOMMUTLBEntry entry; +} IOMMUTLBEvent; + /* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */ #define RAM_PREALLOC (1 << 0) @@ -1326,24 +1331,18 @@ uint64_t memory_region_iommu_get_min_page_size(IOMMUMemoryRegion *iommu_mr); /** * memory_region_notify_iommu: notify a change in an IOMMU translation entry. * - * The notification type will be decided by entry.perm bits: - * - * - For UNMAP (cache invalidation) notifies: set entry.perm to IOMMU_NONE. - * - For MAP (newly added entry) notifies: set entry.perm to the - * permission of the page (which is definitely !IOMMU_NONE). - * * Note: for any IOMMU implementation, an in-place mapping change * should be notified with an UNMAP followed by a MAP. * * @iommu_mr: the memory region that was changed * @iommu_idx: the IOMMU index for the translation table which has changed - * @entry: the new entry in the IOMMU translation table. The entry - * replaces all old entries for the same virtual I/O address range. - * Deleted entries have .@perm == 0. + * @event: TLB event with the new entry in the IOMMU translation table. + * The entry replaces all old entries for the same virtual I/O address + * range. */ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, int iommu_idx, - IOMMUTLBEntry entry); + IOMMUTLBEvent event); /** * memory_region_notify_iommu_one: notify a change in an IOMMU translation @@ -1353,12 +1352,12 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, * notifies a specific notifier, not all of them. * * @notifier: the notifier to be notified - * @entry: the new entry in the IOMMU translation table. The entry - * replaces all old entries for the same virtual I/O address range. - * Deleted entries have .@perm == 0. + * @event: TLB event with the new entry in the IOMMU translation table. + * The entry replaces all old entries for the same virtual I/O address + * range. */ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, - IOMMUTLBEntry *entry); + IOMMUTLBEvent *event); /** * memory_region_register_iommu_notifier: register a notifier for changes to diff --git a/softmmu/memory.c b/softmmu/memory.c index 44de610c72..6ca87e8d73 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1943,11 +1943,15 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, } void memory_region_notify_iommu_one(IOMMUNotifier *notifier, - IOMMUTLBEntry *entry) + IOMMUTLBEvent *event) { - IOMMUNotifierFlag request_flags; + IOMMUTLBEntry *entry = &event->entry; hwaddr entry_end = entry->iova + entry->addr_mask; + if (event->type == IOMMU_NOTIFIER_UNMAP) { + assert(entry->perm == IOMMU_NONE); + } + /* * Skip the notification if the notification does not overlap * with registered range. @@ -1958,20 +1962,14 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, assert(entry->iova >= notifier->start && entry_end <= notifier->end); - if (entry->perm & IOMMU_RW) { - request_flags = IOMMU_NOTIFIER_MAP; - } else { - request_flags = IOMMU_NOTIFIER_UNMAP; - } - - if (notifier->notifier_flags & request_flags) { + if (event->type & notifier->notifier_flags) { notifier->notify(notifier, entry); } } void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, int iommu_idx, - IOMMUTLBEntry entry) + IOMMUTLBEvent event) { IOMMUNotifier *iommu_notifier; @@ -1979,7 +1977,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, IOMMU_NOTIFIER_FOREACH(iommu_notifier, iommu_mr) { if (iommu_notifier->iommu_idx == iommu_idx) { - memory_region_notify_iommu_one(iommu_notifier, &entry); + memory_region_notify_iommu_one(iommu_notifier, &event); } } } From b68ba1ca57677acf870d5ab10579e6105c1f5338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eugenio=20P=C3=A9rez?= Date: Mon, 16 Nov 2020 17:55:04 +0100 Subject: [PATCH 04/65] memory: Add IOMMU_NOTIFIER_DEVIOTLB_UNMAP IOMMUTLBNotificationType MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to differentiate between regular IOMMU map/unmap events and DEVIOTLB unmap. Doing so, notifiers that only need device IOTLB invalidations will not receive regular IOMMU unmappings. Adapt intel and vhost to use it. Signed-off-by: Eugenio Pérez Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Acked-by: Jason Wang Message-Id: <20201116165506.31315-4-eperezma@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 2 +- hw/virtio/vhost.c | 2 +- include/exec/memory.h | 7 ++++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 56180b1c43..edc3090f91 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2468,7 +2468,7 @@ static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s, sz = VTD_PAGE_SIZE; } - event.type = IOMMU_NOTIFIER_UNMAP; + event.type = IOMMU_NOTIFIER_DEVIOTLB_UNMAP; event.entry.target_as = &vtd_dev_as->as; event.entry.addr_mask = sz - 1; event.entry.iova = addr; diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 614ccc2bcb..28c7d78172 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -718,7 +718,7 @@ static void vhost_iommu_region_add(MemoryListener *listener, iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr, MEMTXATTRS_UNSPECIFIED); iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify, - IOMMU_NOTIFIER_UNMAP, + IOMMU_NOTIFIER_DEVIOTLB_UNMAP, section->offset_within_region, int128_get64(end), iommu_idx); diff --git a/include/exec/memory.h b/include/exec/memory.h index e86b5e92da..521d9901d7 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -97,9 +97,14 @@ typedef enum { IOMMU_NOTIFIER_UNMAP = 0x1, /* Notify entry changes (newly created entries) */ IOMMU_NOTIFIER_MAP = 0x2, + /* Notify changes on device IOTLB entries */ + IOMMU_NOTIFIER_DEVIOTLB_UNMAP = 0x04, } IOMMUNotifierFlag; -#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP) +#define IOMMU_NOTIFIER_IOTLB_EVENTS (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP) +#define IOMMU_NOTIFIER_DEVIOTLB_EVENTS IOMMU_NOTIFIER_DEVIOTLB_UNMAP +#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_IOTLB_EVENTS | \ + IOMMU_NOTIFIER_DEVIOTLB_EVENTS) struct IOMMUNotifier; typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier, From f7701e2c7983b680790af47117577b285b6a1aed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eugenio=20P=C3=A9rez?= Date: Mon, 16 Nov 2020 17:55:05 +0100 Subject: [PATCH 05/65] intel_iommu: Skip page walking on device iotlb invalidations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although they didn't reach the notifier because of the filtering in memory_region_notify_iommu_one, the vt-d was still splitting huge memory invalidations in chunks. Skipping it. This improves performance in case of netperf with vhost-net: * TCP_STREAM: From 1923.6Mbit/s to 2175.13Mbit/s (13%) * TCP_RR: From 8464.73 trans/s to 8932.703333 trans/s (5.5%) * UDP_RR: From 8562.08 trans/s to 9005.62/s (5.1%) * UDP_STREAM: No change observed (insignificant 0.1% improvement) Signed-off-by: Eugenio Pérez Acked-by: Jason Wang Message-Id: <20201116165506.31315-5-eperezma@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index edc3090f91..0cc71e4057 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -1478,6 +1478,10 @@ static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as) VTDContextEntry ce; IOMMUNotifier *n; + if (!(vtd_as->iommu.iommu_notify_flags & IOMMU_NOTIFIER_IOTLB_EVENTS)) { + return 0; + } + ret = vtd_dev_to_context_entry(vtd_as->iommu_state, pci_bus_num(vtd_as->bus), vtd_as->devfn, &ce); From 1804857f19f612f6907832e35599cdb51d4ec764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eugenio=20P=C3=A9rez?= Date: Mon, 16 Nov 2020 17:55:06 +0100 Subject: [PATCH 06/65] memory: Skip bad range assertion if notifier is DEVIOTLB_UNMAP type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Device IOTLB invalidations can unmap arbitrary ranges, eiter outside of the memory region or even [0, ~0ULL] for all the space. The assertion could be hit by a guest, and rhel7 guest effectively hit it. Signed-off-by: Eugenio Pérez Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Acked-by: Jason Wang Message-Id: <20201116165506.31315-6-eperezma@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- softmmu/memory.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/softmmu/memory.c b/softmmu/memory.c index 6ca87e8d73..22bacbbc78 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1947,6 +1947,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, { IOMMUTLBEntry *entry = &event->entry; hwaddr entry_end = entry->iova + entry->addr_mask; + IOMMUTLBEntry tmp = *entry; if (event->type == IOMMU_NOTIFIER_UNMAP) { assert(entry->perm == IOMMU_NONE); @@ -1960,10 +1961,16 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, return; } - assert(entry->iova >= notifier->start && entry_end <= notifier->end); + if (notifier->notifier_flags & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) { + /* Crop (iova, addr_mask) to range */ + tmp.iova = MAX(tmp.iova, notifier->start); + tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova; + } else { + assert(entry->iova >= notifier->start && entry_end <= notifier->end); + } if (event->type & notifier->notifier_flags) { - notifier->notify(notifier, entry); + notifier->notify(notifier, &tmp); } } From 4aedda25e883c7c2e7cae911e39b84ad96ef4766 Mon Sep 17 00:00:00 2001 From: John Levon Date: Fri, 20 Nov 2020 18:51:07 +0000 Subject: [PATCH 07/65] virtio: reset device on bad guest index in virtio_load() If we find a queue with an inconsistent guest index value, explicitly mark the device as needing a reset - and broken - via virtio_error(). There's at least one driver implementation - the virtio-win NetKVM driver - that is able to handle a VIRTIO_CONFIG_S_NEEDS_RESET notification and successfully restore the device to a working state. Other implementations do not correctly handle this, but as the VQ is not in a functional state anyway, this is still worth doing. Signed-off-by: John Levon Message-Id: <20201120185103.GA442386@sent> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index ceb58fda6c..eff35fab7c 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -3161,12 +3161,15 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; /* Check it isn't doing strange things with descriptor numbers. */ if (nheads > vdev->vq[i].vring.num) { - qemu_log_mask(LOG_GUEST_ERROR, - "VQ %d size 0x%x Guest index 0x%x " - "inconsistent with Host index 0x%x: delta 0x%x", - i, vdev->vq[i].vring.num, - vring_avail_idx(&vdev->vq[i]), - vdev->vq[i].last_avail_idx, nheads); + virtio_error(vdev, "VQ %d size 0x%x Guest index 0x%x " + "inconsistent with Host index 0x%x: delta 0x%x", + i, vdev->vq[i].vring.num, + vring_avail_idx(&vdev->vq[i]), + vdev->vq[i].last_avail_idx, nheads); + vdev->vq[i].used_idx = 0; + vdev->vq[i].shadow_avail_idx = 0; + vdev->vq[i].inuse = 0; + continue; } vdev->vq[i].used_idx = vring_used_idx(&vdev->vq[i]); vdev->vq[i].shadow_avail_idx = vring_avail_idx(&vdev->vq[i]); From a0e2905b4106296eba396630d37bd3f146e721e1 Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:33 +0800 Subject: [PATCH 08/65] acpi/gpex: Extract two APIs from acpi_dsdt_add_pci Extract two APIs acpi_dsdt_add_pci_route_table and acpi_dsdt_add_pci_osc from acpi_dsdt_add_pci. The first API is used to specify the pci route table and the second API is used to declare the operation system capabilities. These two APIs would be used to specify the pxb-pcie in DSDT. Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-2-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci-host/gpex-acpi.c | 112 ++++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 49 deletions(-) diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c index dbb350a837..32a9f2796d 100644 --- a/hw/pci-host/gpex-acpi.c +++ b/hw/pci-host/gpex-acpi.c @@ -2,21 +2,11 @@ #include "hw/acpi/aml-build.h" #include "hw/pci-host/gpex.h" -void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) +static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq) { - int nr_pcie_buses = cfg->ecam.size / PCIE_MMCFG_SIZE_MIN; - Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf; + Aml *method, *crs; int i, slot_no; - Aml *dev = aml_device("%s", "PCI0"); - aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08"))); - aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); - aml_append(dev, aml_name_decl("_SEG", aml_int(0))); - aml_append(dev, aml_name_decl("_BBN", aml_int(0))); - aml_append(dev, aml_name_decl("_UID", aml_int(0))); - aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device"))); - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); - /* Declare the PCI Routing Table. */ Aml *rt_pkg = aml_varpackage(PCI_SLOT_MAX * PCI_NUM_PINS); for (slot_no = 0; slot_no < PCI_SLOT_MAX; slot_no++) { @@ -34,7 +24,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) /* Create GSI link device */ for (i = 0; i < PCI_NUM_PINS; i++) { - uint32_t irqs = cfg->irq + i; + uint32_t irqs = irq + i; Aml *dev_gsi = aml_device("GSI%d", i); aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F"))); aml_append(dev_gsi, aml_name_decl("_UID", aml_int(i))); @@ -52,43 +42,11 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) aml_append(dev_gsi, method); aml_append(dev, dev_gsi); } +} - method = aml_method("_CBA", 0, AML_NOTSERIALIZED); - aml_append(method, aml_return(aml_int(cfg->ecam.base))); - aml_append(dev, method); - - Aml *rbuf = aml_resource_template(); - aml_append(rbuf, - aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, - 0x0000, 0x0000, nr_pcie_buses - 1, 0x0000, - nr_pcie_buses)); - if (cfg->mmio32.size) { - aml_append(rbuf, - aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED, - AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, - cfg->mmio32.base, - cfg->mmio32.base + cfg->mmio32.size - 1, - 0x0000, - cfg->mmio32.size)); - } - if (cfg->pio.size) { - aml_append(rbuf, - aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, - AML_ENTIRE_RANGE, 0x0000, 0x0000, - cfg->pio.size - 1, - cfg->pio.base, - cfg->pio.size)); - } - if (cfg->mmio64.size) { - aml_append(rbuf, - aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED, - AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, - cfg->mmio64.base, - cfg->mmio64.base + cfg->mmio64.size - 1, - 0x0000, - cfg->mmio64.size)); - } - aml_append(dev, aml_name_decl("_CRS", rbuf)); +static void acpi_dsdt_add_pci_osc(Aml *dev) +{ + Aml *method, *UUID, *ifctx, *ifctx1, *elsectx, *buf; /* Declare an _OSC (OS Control Handoff) method */ aml_append(dev, aml_name_decl("SUPP", aml_int(0))); @@ -160,6 +118,62 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) buf = aml_buffer(1, byte_list); aml_append(method, aml_return(buf)); aml_append(dev, method); +} + +void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) +{ + int nr_pcie_buses = cfg->ecam.size / PCIE_MMCFG_SIZE_MIN; + Aml *method, *crs, *dev, *rbuf; + + dev = aml_device("%s", "PCI0"); + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08"))); + aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); + aml_append(dev, aml_name_decl("_SEG", aml_int(0))); + aml_append(dev, aml_name_decl("_BBN", aml_int(0))); + aml_append(dev, aml_name_decl("_UID", aml_int(0))); + aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device"))); + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); + + acpi_dsdt_add_pci_route_table(dev, cfg->irq); + + method = aml_method("_CBA", 0, AML_NOTSERIALIZED); + aml_append(method, aml_return(aml_int(cfg->ecam.base))); + aml_append(dev, method); + + rbuf = aml_resource_template(); + aml_append(rbuf, + aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, + 0x0000, 0x0000, nr_pcie_buses - 1, 0x0000, + nr_pcie_buses)); + if (cfg->mmio32.size) { + aml_append(rbuf, + aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED, + AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, + cfg->mmio32.base, + cfg->mmio32.base + cfg->mmio32.size - 1, + 0x0000, + cfg->mmio32.size)); + } + if (cfg->pio.size) { + aml_append(rbuf, + aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, + AML_ENTIRE_RANGE, 0x0000, 0x0000, + cfg->pio.size - 1, + cfg->pio.base, + cfg->pio.size)); + } + if (cfg->mmio64.size) { + aml_append(rbuf, + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED, + AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, + cfg->mmio64.base, + cfg->mmio64.base + cfg->mmio64.size - 1, + 0x0000, + cfg->mmio64.size)); + } + aml_append(dev, aml_name_decl("_CRS", rbuf)); + + acpi_dsdt_add_pci_osc(dev); Aml *dev_res0 = aml_device("%s", "RES0"); aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02"))); From 0abd38885ac0fcdb08653922f339849cad387961 Mon Sep 17 00:00:00 2001 From: Jiahui Cen Date: Thu, 19 Nov 2020 09:48:34 +0800 Subject: [PATCH 09/65] fw_cfg: Refactor extra pci roots addition Extract extra pci roots addition from pc machine, which could be used by other machines. In order to make uefi get the extra roots, it is necessary to write extra roots into fw_cfg. And only if the uefi knows there are extra roots, the config spaces of devices behind the root could be obtained. Signed-off-by: Jiahui Cen Signed-off-by: Yubo Miao Message-Id: <20201119014841.7298-3-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 18 +----------------- hw/nvram/fw_cfg.c | 23 +++++++++++++++++++++++ include/hw/nvram/fw_cfg.h | 9 +++++++++ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 17b514d1da..76a846ff9a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -777,27 +777,11 @@ void pc_machine_done(Notifier *notifier, void *data) PCMachineState *pcms = container_of(notifier, PCMachineState, machine_done); X86MachineState *x86ms = X86_MACHINE(pcms); - PCIBus *bus = pcms->bus; /* set the number of CPUs */ x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus); - if (bus) { - int extra_hosts = 0; - - QLIST_FOREACH(bus, &bus->child, sibling) { - /* look for expander root buses */ - if (pci_bus_is_root(bus)) { - extra_hosts++; - } - } - if (extra_hosts && x86ms->fw_cfg) { - uint64_t *val = g_malloc(sizeof(*val)); - *val = cpu_to_le64(extra_hosts); - fw_cfg_add_file(x86ms->fw_cfg, - "etc/extra-pci-roots", val, sizeof(*val)); - } - } + fw_cfg_add_extra_pci_roots(pcms->bus, x86ms->fw_cfg); acpi_setup(); if (x86ms->fw_cfg) { diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 08539a1aab..282ba93e2e 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -40,6 +40,7 @@ #include "qemu/cutils.h" #include "qapi/error.h" #include "hw/acpi/aml-build.h" +#include "hw/pci/pci_bus.h" #define FW_CFG_FILE_SLOTS_DFLT 0x20 @@ -1061,6 +1062,28 @@ bool fw_cfg_add_from_generator(FWCfgState *s, const char *filename, return true; } +void fw_cfg_add_extra_pci_roots(PCIBus *bus, FWCfgState *s) +{ + int extra_hosts = 0; + + if (!bus) { + return; + } + + QLIST_FOREACH(bus, &bus->child, sibling) { + /* look for expander root buses */ + if (pci_bus_is_root(bus)) { + extra_hosts++; + } + } + + if (extra_hosts && s) { + uint64_t *val = g_malloc(sizeof(*val)); + *val = cpu_to_le64(extra_hosts); + fw_cfg_add_file(s, "etc/extra-pci-roots", val, sizeof(*val)); + } +} + static void fw_cfg_machine_reset(void *opaque) { MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 8a9f5738bf..0e7a8bc7af 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -308,6 +308,15 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data, bool fw_cfg_add_from_generator(FWCfgState *s, const char *filename, const char *gen_id, Error **errp); +/** + * fw_cfg_add_extra_pci_roots: + * @bus: main pci root bus to be scanned from + * @s: fw_cfg device being modified + * + * Add a new fw_cfg item... + */ +void fw_cfg_add_extra_pci_roots(PCIBus *bus, FWCfgState *s); + FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, AddressSpace *dma_as); FWCfgState *fw_cfg_init_io(uint32_t iobase); From 09fad16744480938543c0e39cfbaecbbd162c39b Mon Sep 17 00:00:00 2001 From: Jiahui Cen Date: Thu, 19 Nov 2020 09:48:35 +0800 Subject: [PATCH 10/65] hw/arm/virt: Write extra pci roots into fw_cfg Add bus property to virt machine for primary PCI root bus and use it to add extra pci roots behind it. Signed-off-by: Jiahui Cen Signed-off-by: Yubo Miao Message-Id: <20201119014841.7298-4-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/virt.c | 7 +++++-- include/hw/arm/virt.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 27dbeb549e..847257aa5c 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1289,7 +1289,8 @@ static void create_pcie(VirtMachineState *vms) } pci = PCI_HOST_BRIDGE(dev); - if (pci->bus) { + vms->bus = pci->bus; + if (vms->bus) { for (i = 0; i < nb_nics; i++) { NICInfo *nd = &nd_table[i]; @@ -1346,7 +1347,7 @@ static void create_pcie(VirtMachineState *vms) switch (vms->iommu) { case VIRT_IOMMU_SMMUV3: - create_smmu(vms, pci->bus); + create_smmu(vms, vms->bus); qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map", 0x0, vms->iommu_phandle, 0x0, 0x10000); break; @@ -1481,6 +1482,8 @@ void virt_machine_done(Notifier *notifier, void *data) exit(1); } + fw_cfg_add_extra_pci_roots(vms->bus, vms->fw_cfg); + virt_acpi_setup(vms); virt_build_smbios(vms); } diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index aad6d69841..abf54fab49 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -163,6 +163,7 @@ struct VirtMachineState { DeviceState *gic; DeviceState *acpi_dev; Notifier powerdown_notifier; + PCIBus *bus; }; #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM) From 37d5c0a8ff812682c50865cd314348611319d872 Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:36 +0800 Subject: [PATCH 11/65] acpi: Extract crs build form acpi_build.c Extract crs build form acpi_build.c, the function could also be used to build the crs for pxbs for arm. The resources are composed by two parts: 1. The bar space of pci-bridge/pcie-root-ports 2. The resources needed by devices behind PXBs. The base and limit of memory/io are obtained from the config via two APIs: pci_bridge_get_base and pci_bridge_get_limit Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-5-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/aml-build.c | 285 +++++++++++++++++++++++++++++++++++ hw/i386/acpi-build.c | 293 ------------------------------------ include/hw/acpi/aml-build.h | 22 +++ 3 files changed, 307 insertions(+), 293 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 3792ba96ce..f976aa667b 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -27,6 +27,9 @@ #include "sysemu/numa.h" #include "hw/boards.h" #include "hw/acpi/tpm.h" +#include "hw/pci/pci_host.h" +#include "hw/pci/pci_bus.h" +#include "hw/pci/pci_bridge.h" static GArray *build_alloc_array(void) { @@ -55,6 +58,128 @@ static void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 +void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit) +{ + CrsRangeEntry *entry; + + entry = g_malloc(sizeof(*entry)); + entry->base = base; + entry->limit = limit; + + g_ptr_array_add(ranges, entry); +} + +static void crs_range_free(gpointer data) +{ + CrsRangeEntry *entry = (CrsRangeEntry *)data; + g_free(entry); +} + +void crs_range_set_init(CrsRangeSet *range_set) +{ + range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free); + range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free); + range_set->mem_64bit_ranges = + g_ptr_array_new_with_free_func(crs_range_free); +} + +void crs_range_set_free(CrsRangeSet *range_set) +{ + g_ptr_array_free(range_set->io_ranges, true); + g_ptr_array_free(range_set->mem_ranges, true); + g_ptr_array_free(range_set->mem_64bit_ranges, true); +} + +static gint crs_range_compare(gconstpointer a, gconstpointer b) +{ + CrsRangeEntry *entry_a = *(CrsRangeEntry **)a; + CrsRangeEntry *entry_b = *(CrsRangeEntry **)b; + + if (entry_a->base < entry_b->base) { + return -1; + } else if (entry_a->base > entry_b->base) { + return 1; + } else { + return 0; + } +} + +/* + * crs_replace_with_free_ranges - given the 'used' ranges within [start - end] + * interval, computes the 'free' ranges from the same interval. + * Example: If the input array is { [a1 - a2],[b1 - b2] }, the function + * will return { [base - a1], [a2 - b1], [b2 - limit] }. + */ +void crs_replace_with_free_ranges(GPtrArray *ranges, + uint64_t start, uint64_t end) +{ + GPtrArray *free_ranges = g_ptr_array_new(); + uint64_t free_base = start; + int i; + + g_ptr_array_sort(ranges, crs_range_compare); + for (i = 0; i < ranges->len; i++) { + CrsRangeEntry *used = g_ptr_array_index(ranges, i); + + if (free_base < used->base) { + crs_range_insert(free_ranges, free_base, used->base - 1); + } + + free_base = used->limit + 1; + } + + if (free_base < end) { + crs_range_insert(free_ranges, free_base, end); + } + + g_ptr_array_set_size(ranges, 0); + for (i = 0; i < free_ranges->len; i++) { + g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i)); + } + + g_ptr_array_free(free_ranges, true); +} + +/* + * crs_range_merge - merges adjacent ranges in the given array. + * Array elements are deleted and replaced with the merged ranges. + */ +static void crs_range_merge(GPtrArray *range) +{ + GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free); + CrsRangeEntry *entry; + uint64_t range_base, range_limit; + int i; + + if (!range->len) { + return; + } + + g_ptr_array_sort(range, crs_range_compare); + + entry = g_ptr_array_index(range, 0); + range_base = entry->base; + range_limit = entry->limit; + for (i = 1; i < range->len; i++) { + entry = g_ptr_array_index(range, i); + if (entry->base - 1 == range_limit) { + range_limit = entry->limit; + } else { + crs_range_insert(tmp, range_base, range_limit); + range_base = entry->base; + range_limit = entry->limit; + } + } + crs_range_insert(tmp, range_base, range_limit); + + g_ptr_array_set_size(range, 0); + for (i = 0; i < tmp->len; i++) { + entry = g_ptr_array_index(tmp, i); + crs_range_insert(range, entry->base, entry->limit); + } + g_ptr_array_free(tmp, true); +} + static void build_append_nameseg(GArray *array, const char *seg) { @@ -1951,6 +2076,166 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog) tpm2_ptr, "TPM2", table_data->len - tpm2_start, 4, NULL, NULL); } +Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set) +{ + Aml *crs = aml_resource_template(); + CrsRangeSet temp_range_set; + CrsRangeEntry *entry; + uint8_t max_bus = pci_bus_num(host->bus); + uint8_t type; + int devfn; + int i; + + crs_range_set_init(&temp_range_set); + for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) { + uint64_t range_base, range_limit; + PCIDevice *dev = host->bus->devices[devfn]; + + if (!dev) { + continue; + } + + for (i = 0; i < PCI_NUM_REGIONS; i++) { + PCIIORegion *r = &dev->io_regions[i]; + + range_base = r->addr; + range_limit = r->addr + r->size - 1; + + /* + * Work-around for old bioses + * that do not support multiple root buses + */ + if (!range_base || range_base > range_limit) { + continue; + } + + if (r->type & PCI_BASE_ADDRESS_SPACE_IO) { + crs_range_insert(temp_range_set.io_ranges, + range_base, range_limit); + } else { /* "memory" */ + uint64_t length = range_limit - range_base + 1; + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { + crs_range_insert(temp_range_set.mem_ranges, range_base, + range_limit); + } else { + crs_range_insert(temp_range_set.mem_64bit_ranges, + range_base, range_limit); + } + } + } + + type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION; + if (type == PCI_HEADER_TYPE_BRIDGE) { + uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS]; + if (subordinate > max_bus) { + max_bus = subordinate; + } + + range_base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO); + range_limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO); + + /* + * Work-around for old bioses + * that do not support multiple root buses + */ + if (range_base && range_base <= range_limit) { + crs_range_insert(temp_range_set.io_ranges, + range_base, range_limit); + } + + range_base = + pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY); + range_limit = + pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY); + + /* + * Work-around for old bioses + * that do not support multiple root buses + */ + if (range_base && range_base <= range_limit) { + uint64_t length = range_limit - range_base + 1; + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { + crs_range_insert(temp_range_set.mem_ranges, + range_base, range_limit); + } else { + crs_range_insert(temp_range_set.mem_64bit_ranges, + range_base, range_limit); + } + } + + range_base = + pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH); + range_limit = + pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH); + + /* + * Work-around for old bioses + * that do not support multiple root buses + */ + if (range_base && range_base <= range_limit) { + uint64_t length = range_limit - range_base + 1; + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { + crs_range_insert(temp_range_set.mem_ranges, + range_base, range_limit); + } else { + crs_range_insert(temp_range_set.mem_64bit_ranges, + range_base, range_limit); + } + } + } + } + + crs_range_merge(temp_range_set.io_ranges); + for (i = 0; i < temp_range_set.io_ranges->len; i++) { + entry = g_ptr_array_index(temp_range_set.io_ranges, i); + aml_append(crs, + aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED, + AML_POS_DECODE, AML_ENTIRE_RANGE, + 0, entry->base, entry->limit, 0, + entry->limit - entry->base + 1)); + crs_range_insert(range_set->io_ranges, entry->base, entry->limit); + } + + crs_range_merge(temp_range_set.mem_ranges); + for (i = 0; i < temp_range_set.mem_ranges->len; i++) { + entry = g_ptr_array_index(temp_range_set.mem_ranges, i); + assert(entry->limit <= UINT32_MAX && + (entry->limit - entry->base + 1) <= UINT32_MAX); + aml_append(crs, + aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, + AML_MAX_FIXED, AML_NON_CACHEABLE, + AML_READ_WRITE, + 0, entry->base, entry->limit, 0, + entry->limit - entry->base + 1)); + crs_range_insert(range_set->mem_ranges, entry->base, entry->limit); + } + + crs_range_merge(temp_range_set.mem_64bit_ranges); + for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) { + entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i); + aml_append(crs, + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, + AML_MAX_FIXED, AML_NON_CACHEABLE, + AML_READ_WRITE, + 0, entry->base, entry->limit, 0, + entry->limit - entry->base + 1)); + crs_range_insert(range_set->mem_64bit_ranges, + entry->base, entry->limit); + } + + crs_range_set_free(&temp_range_set); + + aml_append(crs, + aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, + 0, + pci_bus_num(host->bus), + max_bus, + 0, + max_bus - pci_bus_num(host->bus) + 1)); + + return crs; +} + /* ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors */ static Aml *aml_serial_bus_device(uint8_t serial_bus_type, uint8_t flags, uint16_t type_flags, diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 1f5c211245..76e27f8fad 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -613,299 +613,6 @@ static Aml *build_prt(bool is_pci0_prt) return method; } -typedef struct CrsRangeEntry { - uint64_t base; - uint64_t limit; -} CrsRangeEntry; - -static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit) -{ - CrsRangeEntry *entry; - - entry = g_malloc(sizeof(*entry)); - entry->base = base; - entry->limit = limit; - - g_ptr_array_add(ranges, entry); -} - -static void crs_range_free(gpointer data) -{ - CrsRangeEntry *entry = (CrsRangeEntry *)data; - g_free(entry); -} - -typedef struct CrsRangeSet { - GPtrArray *io_ranges; - GPtrArray *mem_ranges; - GPtrArray *mem_64bit_ranges; - } CrsRangeSet; - -static void crs_range_set_init(CrsRangeSet *range_set) -{ - range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free); - range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free); - range_set->mem_64bit_ranges = - g_ptr_array_new_with_free_func(crs_range_free); -} - -static void crs_range_set_free(CrsRangeSet *range_set) -{ - g_ptr_array_free(range_set->io_ranges, true); - g_ptr_array_free(range_set->mem_ranges, true); - g_ptr_array_free(range_set->mem_64bit_ranges, true); -} - -static gint crs_range_compare(gconstpointer a, gconstpointer b) -{ - CrsRangeEntry *entry_a = *(CrsRangeEntry **)a; - CrsRangeEntry *entry_b = *(CrsRangeEntry **)b; - - if (entry_a->base < entry_b->base) { - return -1; - } else if (entry_a->base > entry_b->base) { - return 1; - } else { - return 0; - } -} - -/* - * crs_replace_with_free_ranges - given the 'used' ranges within [start - end] - * interval, computes the 'free' ranges from the same interval. - * Example: If the input array is { [a1 - a2],[b1 - b2] }, the function - * will return { [base - a1], [a2 - b1], [b2 - limit] }. - */ -static void crs_replace_with_free_ranges(GPtrArray *ranges, - uint64_t start, uint64_t end) -{ - GPtrArray *free_ranges = g_ptr_array_new(); - uint64_t free_base = start; - int i; - - g_ptr_array_sort(ranges, crs_range_compare); - for (i = 0; i < ranges->len; i++) { - CrsRangeEntry *used = g_ptr_array_index(ranges, i); - - if (free_base < used->base) { - crs_range_insert(free_ranges, free_base, used->base - 1); - } - - free_base = used->limit + 1; - } - - if (free_base < end) { - crs_range_insert(free_ranges, free_base, end); - } - - g_ptr_array_set_size(ranges, 0); - for (i = 0; i < free_ranges->len; i++) { - g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i)); - } - - g_ptr_array_free(free_ranges, true); -} - -/* - * crs_range_merge - merges adjacent ranges in the given array. - * Array elements are deleted and replaced with the merged ranges. - */ -static void crs_range_merge(GPtrArray *range) -{ - GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free); - CrsRangeEntry *entry; - uint64_t range_base, range_limit; - int i; - - if (!range->len) { - return; - } - - g_ptr_array_sort(range, crs_range_compare); - - entry = g_ptr_array_index(range, 0); - range_base = entry->base; - range_limit = entry->limit; - for (i = 1; i < range->len; i++) { - entry = g_ptr_array_index(range, i); - if (entry->base - 1 == range_limit) { - range_limit = entry->limit; - } else { - crs_range_insert(tmp, range_base, range_limit); - range_base = entry->base; - range_limit = entry->limit; - } - } - crs_range_insert(tmp, range_base, range_limit); - - g_ptr_array_set_size(range, 0); - for (i = 0; i < tmp->len; i++) { - entry = g_ptr_array_index(tmp, i); - crs_range_insert(range, entry->base, entry->limit); - } - g_ptr_array_free(tmp, true); -} - -static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set) -{ - Aml *crs = aml_resource_template(); - CrsRangeSet temp_range_set; - CrsRangeEntry *entry; - uint8_t max_bus = pci_bus_num(host->bus); - uint8_t type; - int devfn; - int i; - - crs_range_set_init(&temp_range_set); - for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) { - uint64_t range_base, range_limit; - PCIDevice *dev = host->bus->devices[devfn]; - - if (!dev) { - continue; - } - - for (i = 0; i < PCI_NUM_REGIONS; i++) { - PCIIORegion *r = &dev->io_regions[i]; - - range_base = r->addr; - range_limit = r->addr + r->size - 1; - - /* - * Work-around for old bioses - * that do not support multiple root buses - */ - if (!range_base || range_base > range_limit) { - continue; - } - - if (r->type & PCI_BASE_ADDRESS_SPACE_IO) { - crs_range_insert(temp_range_set.io_ranges, - range_base, range_limit); - } else { /* "memory" */ - uint64_t length = range_limit - range_base + 1; - if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { - crs_range_insert(temp_range_set.mem_ranges, range_base, - range_limit); - } else { - crs_range_insert(temp_range_set.mem_64bit_ranges, - range_base, range_limit); - } - } - } - - type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION; - if (type == PCI_HEADER_TYPE_BRIDGE) { - uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS]; - if (subordinate > max_bus) { - max_bus = subordinate; - } - - range_base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO); - range_limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO); - - /* - * Work-around for old bioses - * that do not support multiple root buses - */ - if (range_base && range_base <= range_limit) { - crs_range_insert(temp_range_set.io_ranges, - range_base, range_limit); - } - - range_base = - pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY); - range_limit = - pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY); - - /* - * Work-around for old bioses - * that do not support multiple root buses - */ - if (range_base && range_base <= range_limit) { - uint64_t length = range_limit - range_base + 1; - if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { - crs_range_insert(temp_range_set.mem_ranges, - range_base, range_limit); - } else { - crs_range_insert(temp_range_set.mem_64bit_ranges, - range_base, range_limit); - } - } - - range_base = - pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH); - range_limit = - pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH); - - /* - * Work-around for old bioses - * that do not support multiple root buses - */ - if (range_base && range_base <= range_limit) { - uint64_t length = range_limit - range_base + 1; - if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { - crs_range_insert(temp_range_set.mem_ranges, - range_base, range_limit); - } else { - crs_range_insert(temp_range_set.mem_64bit_ranges, - range_base, range_limit); - } - } - } - } - - crs_range_merge(temp_range_set.io_ranges); - for (i = 0; i < temp_range_set.io_ranges->len; i++) { - entry = g_ptr_array_index(temp_range_set.io_ranges, i); - aml_append(crs, - aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED, - AML_POS_DECODE, AML_ENTIRE_RANGE, - 0, entry->base, entry->limit, 0, - entry->limit - entry->base + 1)); - crs_range_insert(range_set->io_ranges, entry->base, entry->limit); - } - - crs_range_merge(temp_range_set.mem_ranges); - for (i = 0; i < temp_range_set.mem_ranges->len; i++) { - entry = g_ptr_array_index(temp_range_set.mem_ranges, i); - assert(entry->limit <= UINT32_MAX && - (entry->limit - entry->base + 1) <= UINT32_MAX); - aml_append(crs, - aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, - AML_MAX_FIXED, AML_NON_CACHEABLE, - AML_READ_WRITE, - 0, entry->base, entry->limit, 0, - entry->limit - entry->base + 1)); - crs_range_insert(range_set->mem_ranges, entry->base, entry->limit); - } - - crs_range_merge(temp_range_set.mem_64bit_ranges); - for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) { - entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i); - aml_append(crs, - aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, - AML_MAX_FIXED, AML_NON_CACHEABLE, - AML_READ_WRITE, - 0, entry->base, entry->limit, 0, - entry->limit - entry->base + 1)); - crs_range_insert(range_set->mem_64bit_ranges, - entry->base, entry->limit); - } - - crs_range_set_free(&temp_range_set); - - aml_append(crs, - aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE, - 0, - pci_bus_num(host->bus), - max_bus, - 0, - max_bus - pci_bus_num(host->bus) + 1)); - - return crs; -} - static void build_hpet_aml(Aml *table) { Aml *crs; diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index fe0055fffb..e727bea1bc 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -224,6 +224,20 @@ struct AcpiBuildTables { BIOSLinker *linker; } AcpiBuildTables; +typedef +struct CrsRangeEntry { + uint64_t base; + uint64_t limit; +} CrsRangeEntry; + +typedef +struct CrsRangeSet { + GPtrArray *io_ranges; + GPtrArray *mem_ranges; + GPtrArray *mem_64bit_ranges; +} CrsRangeSet; + + /* * ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors * Serial Bus Type @@ -432,6 +446,14 @@ build_append_gas_from_struct(GArray *table, const struct AcpiGenericAddress *s) s->access_width, s->address); } +void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit); +void crs_replace_with_free_ranges(GPtrArray *ranges, + uint64_t start, uint64_t end); +void crs_range_set_init(CrsRangeSet *range_set); +void crs_range_set_free(CrsRangeSet *range_set); + +Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set); + void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, uint64_t len, int node, MemoryAffinityFlags flags); From 6f9765fbad3d86de008f2e0f6821c29155eb0d85 Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:37 +0800 Subject: [PATCH 12/65] acpi/gpex: Build tables for pxb The resources of pxbs are obtained by crs_build and the resources used by pxbs would be moved from the resources defined for host-bridge. The resources for pxb are composed of following two parts: 1. The bar space of the pci-bridge/pcie-root-port behined it 2. The config space of devices behind it. Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-6-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/virt-acpi-build.c | 6 +++-- hw/pci-host/gpex-acpi.c | 54 ++++++++++++++++++++++++++++++++++++++ include/hw/pci-host/gpex.h | 1 + 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 9747a6458f..e0bed9037c 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -153,7 +153,8 @@ static void acpi_dsdt_add_virtio(Aml *scope, } static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, - uint32_t irq, bool use_highmem, bool highmem_ecam) + uint32_t irq, bool use_highmem, bool highmem_ecam, + VirtMachineState *vms) { int ecam_id = VIRT_ECAM_ID(highmem_ecam); struct GPEXConfig cfg = { @@ -161,6 +162,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, .pio = memmap[VIRT_PCIE_PIO], .ecam = memmap[ecam_id], .irq = irq, + .bus = vms->bus, }; if (use_highmem) { @@ -609,7 +611,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO], (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS); acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE), - vms->highmem, vms->highmem_ecam); + vms->highmem, vms->highmem_ecam, vms); if (vms->acpi_dev) { build_ged_aml(scope, "\\_SB."GED_DEVICE, HOTPLUG_HANDLER(vms->acpi_dev), diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c index 32a9f2796d..7f20ee1c98 100644 --- a/hw/pci-host/gpex-acpi.c +++ b/hw/pci-host/gpex-acpi.c @@ -1,6 +1,10 @@ #include "qemu/osdep.h" #include "hw/acpi/aml-build.h" #include "hw/pci-host/gpex.h" +#include "hw/arm/virt.h" +#include "hw/pci/pci_bus.h" +#include "hw/pci/pci_bridge.h" +#include "hw/pci/pcie_host.h" static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq) { @@ -124,7 +128,57 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) { int nr_pcie_buses = cfg->ecam.size / PCIE_MMCFG_SIZE_MIN; Aml *method, *crs, *dev, *rbuf; + PCIBus *bus = cfg->bus; + CrsRangeSet crs_range_set; + /* start to construct the tables for pxb */ + crs_range_set_init(&crs_range_set); + if (bus) { + QLIST_FOREACH(bus, &bus->child, sibling) { + uint8_t bus_num = pci_bus_num(bus); + uint8_t numa_node = pci_bus_numa_node(bus); + + if (!pci_bus_is_root(bus)) { + continue; + } + + /* + * 0 - (nr_pcie_buses - 1) is the bus range for the main + * host-bridge and it equals the MIN of the + * busNr defined for pxb-pcie. + */ + if (bus_num < nr_pcie_buses) { + nr_pcie_buses = bus_num; + } + + dev = aml_device("PC%.02X", bus_num); + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08"))); + aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); + aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num))); + aml_append(dev, aml_name_decl("_UID", aml_int(bus_num))); + aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device"))); + if (numa_node != NUMA_NODE_UNASSIGNED) { + aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node))); + } + + acpi_dsdt_add_pci_route_table(dev, cfg->irq); + + /* + * Resources defined for PXBs are composed by the folling parts: + * 1. The resources the pci-brige/pcie-root-port need. + * 2. The resources the devices behind pxb need. + */ + crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set); + aml_append(dev, aml_name_decl("_CRS", crs)); + + acpi_dsdt_add_pci_osc(dev); + + aml_append(scope, dev); + } + } + crs_range_set_free(&crs_range_set); + + /* tables for the main */ dev = aml_device("%s", "PCI0"); aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08"))); aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03"))); diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h index d52ea80d4e..d48a020a95 100644 --- a/include/hw/pci-host/gpex.h +++ b/include/hw/pci-host/gpex.h @@ -59,6 +59,7 @@ struct GPEXConfig { MemMapEntry mmio64; MemMapEntry pio; int irq; + PCIBus *bus; }; int gpex_set_irq_num(GPEXHost *s, int index, int gsi); From 451b157041d265f94a4be668b9cd234b3e34fabb Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:38 +0800 Subject: [PATCH 13/65] acpi: Align the size to 128k If table size is changed between virt_acpi_build and virt_acpi_build_update, the table size would not be updated to UEFI, therefore, just align the size to 128kb, which is enough and same with x86. It would warn if 64k is not enough and the align size should be updated. Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-7-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/virt-acpi-build.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index e0bed9037c..711cf2069f 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -57,6 +57,8 @@ #define ARM_SPI_BASE 32 +#define ACPI_BUILD_TABLE_SIZE 0x20000 + static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus) { uint16_t i; @@ -656,6 +658,15 @@ struct AcpiBuildState { bool patched; } AcpiBuildState; +static void acpi_align_size(GArray *blob, unsigned align) +{ + /* + * Align size to multiple of given size. This reduces the chance + * we need to change size in the future (breaking cross version migration). + */ + g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align)); +} + static void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) { @@ -743,6 +754,20 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) build_rsdp(tables->rsdp, tables->linker, &rsdp_data); } + /* + * The align size is 128, warn if 64k is not enough therefore + * the align size could be resized. + */ + if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) { + warn_report("ACPI table size %u exceeds %d bytes," + " migration may not work", + tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2); + error_printf("Try removing CPUs, NUMA nodes, memory slots" + " or PCI bridges."); + } + acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE); + + /* Cleanup memory that's no longer used. */ g_array_free(table_offsets, true); } From 128e232281ec0626bfda158f7718921e59e94ac4 Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:39 +0800 Subject: [PATCH 14/65] unit-test: The files changed. The unit-test is seperated into three patches: 1. The files changed and list in bios-tables-test-allowed-diff.h 2. The unit-test 3. The binary file and clear bios-tables-test-allowed-diff.h The ASL diff would also be listed. Sice there are 1000+lines diff, some changes would be omitted. * Original Table Header: * Signature "DSDT" - * Length 0x000014BB (5307) + * Length 0x00001E7A (7802) * Revision 0x02 - * Checksum 0xD1 + * Checksum 0x57 * OEM ID "BOCHS " * OEM Table ID "BXPCDSDT" * OEM Revision 0x00000001 (1) + Device (PC80) + { + Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID + Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID + Name (_ADR, Zero) // _ADR: Address + Name (_CCA, One) // _CCA: Cache Coherency Attribute + Name (_SEG, Zero) // _SEG: PCI Segment + Name (_BBN, 0x80) // _BBN: BIOS Bus Number + Name (_UID, 0x80) // _UID: Unique ID + Name (_STR, Unicode ("pxb Device")) // _STR: Description String + Name (_PRT, Package (0x80) // _PRT: PCI Routing Table + { + Package (0x04) + { + 0xFFFF, + Zero, + GSI0, + Zero + }, + Packages are omitted. + Package (0x04) + { + 0x001FFFFF, + 0x03, + GSI2, + Zero + } + }) + Device (GSI0) + { + Name (_HID, "PNP0C0F" /* PCI Interrupt Link Device */) // _HID: Hardware ID + Name (_UID, Zero) // _UID: Unique ID + Name (_PRS, ResourceTemplate () // _PRS: Possible Resource Settings + { + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) + { + 0x00000023, + } + }) + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) + { + 0x00000023, + } + }) + Method (_SRS, 1, NotSerialized) // _SRS: Set Resource Settings + { + } + } GSI1,2,3 are omitted. + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, + 0x0000, // Granularity + 0x0080, // Range Minimum + 0x0080, // Range Maximum + 0x0000, // Translation Offset + 0x0001, // Length + ,, ) + }) + Name (SUPP, Zero) + Name (CTRL, Zero) + Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities + { + CreateDWordField (Arg3, Zero, CDW1) + If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */)) + { + CreateDWordField (Arg3, 0x04, CDW2) + CreateDWordField (Arg3, 0x08, CDW3) + SUPP = CDW2 /* \_SB_.PC80._OSC.CDW2 */ + CTRL = CDW3 /* \_SB_.PC80._OSC.CDW3 */ + CTRL &= 0x1F + If ((Arg1 != One)) + { + CDW1 |= 0x08 + } + + If ((CDW3 != CTRL)) + { + CDW1 |= 0x10 + } + + CDW3 = CTRL /* \_SB_.PC80.CTRL */ + Return (Arg3) + } + Else + { + CDW1 |= 0x04 + Return (Arg3) + } + } DSM is are omitted Device (PCI0) { Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, 0x0000, // Granularity 0x0000, // Range Minimum - 0x00FF, // Range Maximum + 0x007F, // Range Maximum 0x0000, // Translation Offset - 0x0100, // Length + 0x0080, // Length Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-8-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test-allowed-diff.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..90c53925fc 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/virt/DSDT.pxb", From 1da638b165d5ede4a2079e5072c9b11fc760e9f1 Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:40 +0800 Subject: [PATCH 15/65] unit-test: Add testcase for pxb Add testcase for pxb to make sure the ACPI table is correct for guest. Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-9-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test.c | 58 ++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index f23a5335a8..64a9a772ee 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -671,12 +671,21 @@ static void test_acpi_one(const char *params, test_data *data) * TODO: convert '-drive if=pflash' to new syntax (see e33763be7cd3) * when arm/virt boad starts to support it. */ - args = g_strdup_printf("-machine %s %s -accel tcg -nodefaults -nographic " - "-drive if=pflash,format=raw,file=%s,readonly " - "-drive if=pflash,format=raw,file=%s,snapshot=on -cdrom %s %s", - data->machine, data->tcg_only ? "" : "-accel kvm", - data->uefi_fl1, data->uefi_fl2, data->cd, params ? params : ""); - + if (data->cd) { + args = g_strdup_printf("-machine %s %s -accel tcg " + "-nodefaults -nographic " + "-drive if=pflash,format=raw,file=%s,readonly " + "-drive if=pflash,format=raw,file=%s,snapshot=on -cdrom %s %s", + data->machine, data->tcg_only ? "" : "-accel kvm", + data->uefi_fl1, data->uefi_fl2, data->cd, params ? params : ""); + } else { + args = g_strdup_printf("-machine %s %s -accel tcg " + "-nodefaults -nographic " + "-drive if=pflash,format=raw,file=%s,readonly " + "-drive if=pflash,format=raw,file=%s,snapshot=on %s", + data->machine, data->tcg_only ? "" : "-accel kvm", + data->uefi_fl1, data->uefi_fl2, params ? params : ""); + } } else { args = g_strdup_printf("-machine %s %s -accel tcg " "-net none -display none %s " @@ -1176,6 +1185,40 @@ static void test_acpi_virt_tcg_numamem(void) } +#ifdef CONFIG_PXB +static void test_acpi_virt_tcg_pxb(void) +{ + test_data data = { + .machine = "virt", + .tcg_only = true, + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", + .ram_start = 0x40000000ULL, + .scan_len = 128ULL * 1024 * 1024, + }; + /* + * While using -cdrom, the cdrom would auto plugged into pxb-pcie, + * the reason is the bus of pxb-pcie is also root bus, it would lead + * to the error only PCI/PCIE bridge could plug onto pxb. + * Therefore,thr cdrom is defined and plugged onto the scsi controller + * to solve the conflicts. + */ + data.variant = ".pxb"; + test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" + " -device virtio-scsi-pci,id=scsi0,bus=pci.1" + " -drive file=" + "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," + "if=none,media=cdrom,id=drive-scsi0-0-0-1,readonly=on" + " -device scsi-cd,bus=scsi0.0,scsi-id=0," + "drive=drive-scsi0-0-0-1,id=scsi0-0-0-1,bootindex=1" + " -cpu cortex-a57" + " -device pxb-pcie,bus_nr=128", + &data); + + free_test_data(&data); +} +#endif + static void test_acpi_tcg_acpi_hmat(const char *machine) { test_data data; @@ -1287,6 +1330,9 @@ int main(int argc, char *argv[]) qtest_add_func("acpi/virt", test_acpi_virt_tcg); qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem); qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp); +#ifdef CONFIG_PXB + qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb); +#endif } ret = g_test_run(); boot_sector_cleanup(disk); From fe1127da11549953c0925f5db58f6c5f76c1cec5 Mon Sep 17 00:00:00 2001 From: Yubo Miao Date: Thu, 19 Nov 2020 09:48:41 +0800 Subject: [PATCH 16/65] unit-test: Add the binary file and clear diff.h Add the binary file DSDT.pxb and clear bios-tables-test-allowed-diff.h Signed-off-by: Yubo Miao Signed-off-by: Jiahui Cen Message-Id: <20201119014841.7298-10-cenjiahui@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/virt/DSDT.pxb | Bin 0 -> 7802 bytes tests/qtest/bios-tables-test-allowed-diff.h | 1 - 2 files changed, 1 deletion(-) create mode 100644 tests/data/acpi/virt/DSDT.pxb diff --git a/tests/data/acpi/virt/DSDT.pxb b/tests/data/acpi/virt/DSDT.pxb new file mode 100644 index 0000000000000000000000000000000000000000..d5f0533a02d62bc2ae2db9b9de9484e5c06652fe GIT binary patch literal 7802 zcmeI1%WoT16o;=LiS6+tw&OgUms2Pe&&rRcNlRN|kDbINPK+mQkW$GN2t>&y5*4DY z5GE1@x}%ZUunAHY{255B*s){5x*Prhb`0mvok@O&o()@MN3!S4-1E)-#wYff>!#D( zdAOidc(<`_Z#avMce{3z_Jx#EdRxC{zj_wB({~#Ey~7#1TrS7^8|`MgZg<-hEUS3` zR=cV84zJqVo#0rnvr#TrD*mx}-|jiN8EfisLTO+^WtIANRE0w4D0)D-m9e1W1K-`?I3==%fJX5q(*jFqgf=xI;=+i!j2&*$h#YZ&sEUM@nAgr*&hytUE zjGD-ZNQ_Zn)R1vWWJD!K92l37u_Q7^B!&fyC1hL{8KV*-1&qtcSQZ&EiID-uGBQ>~ zMqFZKfw6*&DG%GO{fw77VxlVHu;{{;Uks;S< zUSgaFMgtjgosLV43&5~}QI+eoATeGBMiUuwolZ!MSAo$&hFqtU661AXtRX|L(Vw8cgfeg7$ixQ&>j5adlI-QXimw<5-8FHP@N{q|EcpDjVoz6*&6<};4 zL$1?#iE$Me9bnYtI$e+$*MPBw47pBA65|FiwtYtDhpxTi&!fB5E!WE{)VJ8wgqf&D zQN7vI`@BBFX|2AGs&X_uAR4$*c+C9j#H9`7}G}OzaP-oI?ys;54Gnhd{>C9kg#AMP?FOx!@Ni*^?sUtLF z{m6IphEmhyTLvL|jxf&=@0@|>h{+5lPa%4aGEZuLX$HYiYO>IiLiCI=&lvNJaZd`- zGtNBYUS@Dfs3}8F3ehvcJgIFrSI@g73GPWDdRolWVxH8*p(lmtnPi?x=9%Q46ryK} zd8U{rHGSwwA$q2nXPSAYxhI9_nPHw8=1EN=dQymU3el5po1kv9%#)f* z^rR3ybIdcxJagQWLiEft&ph*_CKNp>M9*>NInF%CxhI9_Szw+8=1EN}dQym<6U=jh zc}{Ro3ej_tc}_ARl0q^1}>DMZgA^DHvYBKM>a zJ!hEb4D+NW8a*jQ&spX<%RFbfCxz%a$2{klCpF#ZNg;a9GtYVEInO;QL{D1OFrQi8 zXZ!;5q$V9bDMZf_^DHsX68EIgcZYER-{LZqUNJz{O9IR6<1D|`%V{aWZ#O?fGWMl>9uyC1I^JJHH~_tpRAI8KF&Tp zx)=JKj#RwSmE*~$N5MF=JF5>K=)rpb$^MTSvtOU2a0Ek zp0J{OUnX^Ex184IVqw1Dy1kP)(81l~?9rpUmR_}c+}-Uptij%4QEy Date: Wed, 18 Nov 2020 09:37:23 +0100 Subject: [PATCH 17/65] failover: fix indentantion Once there, remove not needed cast. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-3-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 33 +++++++++++++++------------------ softmmu/qdev-monitor.c | 4 ++-- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 9179013ac4..1011a524bf 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -797,7 +797,7 @@ static void failover_add_primary(VirtIONet *n, Error **errp) } n->primary_device_opts = qemu_opts_find(qemu_find_opts("device"), - n->primary_device_id); + n->primary_device_id); if (n->primary_device_opts) { n->primary_dev = qdev_device_add(n->primary_device_opts, &err); if (err) { @@ -814,9 +814,9 @@ static void failover_add_primary(VirtIONet *n, Error **errp) } else { error_setg(errp, "Primary device not found"); error_append_hint(errp, "Virtio-net failover will not work. Make " - "sure primary device has parameter" - " failover_pair_id=\n"); -} + "sure primary device has parameter" + " failover_pair_id=\n"); + } error_propagate(errp, err); } @@ -824,7 +824,6 @@ static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) { VirtIONet *n = opaque; int ret = 0; - const char *standby_id = qemu_opt_get(opts, "failover_pair_id"); if (standby_id != NULL && (g_strcmp0(standby_id, n->netclient_name) == 0)) { @@ -841,14 +840,14 @@ static DeviceState *virtio_net_find_primary(VirtIONet *n, Error **errp) Error *err = NULL; if (qemu_opts_foreach(qemu_find_opts("device"), - is_my_primary, n, &err)) { + is_my_primary, n, &err)) { if (err) { error_propagate(errp, err); return NULL; } if (n->primary_device_id) { dev = qdev_find_recursive(sysbus_get_default(), - n->primary_device_id); + n->primary_device_id); } else { error_setg(errp, "Primary device id not found"); return NULL; @@ -857,8 +856,6 @@ static DeviceState *virtio_net_find_primary(VirtIONet *n, Error **errp) return dev; } - - static DeviceState *virtio_connect_failover_devices(VirtIONet *n, DeviceState *dev, Error **errp) @@ -3126,9 +3123,9 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) return true; } if (!n->primary_device_opts) { - n->primary_device_opts = qemu_opts_from_qdict( - qemu_find_opts("device"), - n->primary_device_dict, errp); + n->primary_device_opts = qemu_opts_from_qdict(qemu_find_opts("device"), + n->primary_device_dict, + errp); if (!n->primary_device_opts) { return false; } @@ -3176,8 +3173,8 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, if (migration_in_setup(s) && !should_be_hidden) { if (failover_unplug_primary(n)) { vmstate_unregister(VMSTATE_IF(n->primary_dev), - qdev_get_vmsd(n->primary_dev), - n->primary_dev); + qdev_get_vmsd(n->primary_dev), + n->primary_dev); qapi_event_send_unplug_primary(n->primary_device_id); qatomic_set(&n->primary_should_be_hidden, true); } else { @@ -3201,7 +3198,7 @@ static void virtio_net_migration_state_notifier(Notifier *notifier, void *data) } static int virtio_net_primary_should_be_hidden(DeviceListener *listener, - QemuOpts *device_opts) + QemuOpts *device_opts) { VirtIONet *n = container_of(listener, VirtIONet, primary_listener); bool match_found = false; @@ -3211,11 +3208,11 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, return -1; } n->primary_device_dict = qemu_opts_to_qdict(device_opts, - n->primary_device_dict); + n->primary_device_dict); if (n->primary_device_dict) { g_free(n->standby_id); n->standby_id = g_strdup(qdict_get_try_str(n->primary_device_dict, - "failover_pair_id")); + "failover_pair_id")); } if (g_strcmp0(n->standby_id, n->netclient_name) == 0) { match_found = true; @@ -3235,7 +3232,7 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, if (n->primary_device_dict) { g_free(n->primary_device_id); n->primary_device_id = g_strdup(qdict_get_try_str( - n->primary_device_dict, "id")); + n->primary_device_dict, "id")); if (!n->primary_device_id) { warn_report("primary_device_id not set"); } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index bf79d0bbcd..a25f5d612c 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -573,10 +573,10 @@ void qdev_set_id(DeviceState *dev, const char *id) } static int is_failover_device(void *opaque, const char *name, const char *value, - Error **errp) + Error **errp) { if (strcmp(name, "failover_pair_id") == 0) { - QemuOpts *opts = (QemuOpts *)opaque; + QemuOpts *opts = opaque; if (qdev_should_hide_device(opts)) { return 1; From 587f2fcb93eddf69736e00731a2da018a0e0a726 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:24 +0100 Subject: [PATCH 18/65] failover: Use always atomics for primary_should_be_hidden Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-4-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 1011a524bf..a0fa63e7cb 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3136,7 +3136,7 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) return false; } qdev_set_parent_bus(n->primary_dev, n->primary_bus, &error_abort); - n->primary_should_be_hidden = false; + qatomic_set(&n->primary_should_be_hidden, false); if (!qemu_opt_set_bool(n->primary_device_opts, "partially_hotplugged", true, errp)) { return false; From 78274682b79d48e8de76c817c67c3cfbb76dc2ee Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:25 +0100 Subject: [PATCH 19/65] failover: primary bus is only used once, and where it is set Just remove the struct member. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-5-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 8 ++++---- include/hw/virtio/virtio-net.h | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index a0fa63e7cb..786d313330 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -804,7 +804,6 @@ static void failover_add_primary(VirtIONet *n, Error **errp) qemu_opts_del(n->primary_device_opts); } if (n->primary_dev) { - n->primary_bus = n->primary_dev->parent_bus; if (err) { qdev_unplug(n->primary_dev, &err); qdev_set_id(n->primary_dev, ""); @@ -3118,6 +3117,7 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) Error *err = NULL; HotplugHandler *hotplug_ctrl; PCIDevice *pdev = PCI_DEVICE(n->primary_dev); + BusState *primary_bus; if (!pdev->partially_hotplugged) { return true; @@ -3130,12 +3130,12 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) return false; } } - n->primary_bus = n->primary_dev->parent_bus; - if (!n->primary_bus) { + primary_bus = n->primary_dev->parent_bus; + if (!primary_bus) { error_setg(errp, "virtio_net: couldn't find primary bus"); return false; } - qdev_set_parent_bus(n->primary_dev, n->primary_bus, &error_abort); + qdev_set_parent_bus(n->primary_dev, primary_bus, &error_abort); qatomic_set(&n->primary_should_be_hidden, false); if (!qemu_opt_set_bool(n->primary_device_opts, "partially_hotplugged", true, errp)) { diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index f4852ac27b..c8da637d40 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -205,7 +205,6 @@ struct VirtIONet { QemuOpts *primary_device_opts; QDict *primary_device_dict; DeviceState *primary_dev; - BusState *primary_bus; char *primary_device_id; char *standby_id; bool primary_should_be_hidden; From 82ceb65799855efb0db965a6ef86d81ae1c8bcd7 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:26 +0100 Subject: [PATCH 20/65] failover: Remove unused parameter Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-6-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 786d313330..3f658d6246 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -855,9 +855,7 @@ static DeviceState *virtio_net_find_primary(VirtIONet *n, Error **errp) return dev; } -static DeviceState *virtio_connect_failover_devices(VirtIONet *n, - DeviceState *dev, - Error **errp) +static DeviceState *virtio_connect_failover_devices(VirtIONet *n, Error **errp) { DeviceState *prim_dev = NULL; Error *err = NULL; @@ -928,7 +926,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) qatomic_set(&n->primary_should_be_hidden, false); failover_add_primary(n, &err); if (err) { - n->primary_dev = virtio_connect_failover_devices(n, n->qdev, &err); + n->primary_dev = virtio_connect_failover_devices(n, &err); if (err) { goto out_err; } @@ -3164,7 +3162,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, should_be_hidden = qatomic_read(&n->primary_should_be_hidden); if (!n->primary_dev) { - n->primary_dev = virtio_connect_failover_devices(n, n->qdev, &err); + n->primary_dev = virtio_connect_failover_devices(n, &err); if (!n->primary_dev) { return; } From 594d308b9314b446ed2ccc42de7b4d57ba1b7118 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:27 +0100 Subject: [PATCH 21/65] failover: Remove external partially_hotplugged property It was only set "once", and with the wrong value. As far as I can see, libvirt still don't use it. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-7-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 3f658d6246..6ca85627d8 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3135,10 +3135,6 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) } qdev_set_parent_bus(n->primary_dev, primary_bus, &error_abort); qatomic_set(&n->primary_should_be_hidden, false); - if (!qemu_opt_set_bool(n->primary_device_opts, - "partially_hotplugged", true, errp)) { - return false; - } hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev); if (hotplug_ctrl) { hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, &err); From 3d1c7a9782d19052505aabc8f2c134ccd6f3f3fb Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:28 +0100 Subject: [PATCH 22/65] failover: qdev_device_add() returns err or dev set Never both. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-8-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 6ca85627d8..3e82108d42 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -803,13 +803,6 @@ static void failover_add_primary(VirtIONet *n, Error **errp) if (err) { qemu_opts_del(n->primary_device_opts); } - if (n->primary_dev) { - if (err) { - qdev_unplug(n->primary_dev, &err); - qdev_set_id(n->primary_dev, ""); - - } - } } else { error_setg(errp, "Primary device not found"); error_append_hint(errp, "Virtio-net failover will not work. Make " From e2bde83e23d3cfc1d90911c74500fd2e3b0b04fa Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:29 +0100 Subject: [PATCH 23/65] failover: Rename bool to failover_primary_hidden You should not use passive naming variables. And once there, be able to search for them. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-9-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 14 +++++++------- include/hw/virtio/virtio-net.h | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 3e82108d42..c221671852 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -916,7 +916,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) { qapi_event_send_failover_negotiated(n->netclient_name); - qatomic_set(&n->primary_should_be_hidden, false); + qatomic_set(&n->failover_primary_hidden, false); failover_add_primary(n, &err); if (err) { n->primary_dev = virtio_connect_failover_devices(n, &err); @@ -3127,7 +3127,7 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) return false; } qdev_set_parent_bus(n->primary_dev, primary_bus, &error_abort); - qatomic_set(&n->primary_should_be_hidden, false); + qatomic_set(&n->failover_primary_hidden, false); hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev); if (hotplug_ctrl) { hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, &err); @@ -3148,7 +3148,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, bool should_be_hidden; Error *err = NULL; - should_be_hidden = qatomic_read(&n->primary_should_be_hidden); + should_be_hidden = qatomic_read(&n->failover_primary_hidden); if (!n->primary_dev) { n->primary_dev = virtio_connect_failover_devices(n, &err); @@ -3163,7 +3163,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, qdev_get_vmsd(n->primary_dev), n->primary_dev); qapi_event_send_unplug_primary(n->primary_device_id); - qatomic_set(&n->primary_should_be_hidden, true); + qatomic_set(&n->failover_primary_hidden, true); } else { warn_report("couldn't unplug primary device"); } @@ -3213,8 +3213,8 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, n->primary_device_opts = device_opts; - /* primary_should_be_hidden is set during feature negotiation */ - hide = qatomic_read(&n->primary_should_be_hidden); + /* failover_primary_hidden is set during feature negotiation */ + hide = qatomic_read(&n->failover_primary_hidden); if (n->primary_device_dict) { g_free(n->primary_device_id); @@ -3271,7 +3271,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) if (n->failover) { n->primary_listener.should_be_hidden = virtio_net_primary_should_be_hidden; - qatomic_set(&n->primary_should_be_hidden, true); + qatomic_set(&n->failover_primary_hidden, true); device_listener_register(&n->primary_listener); n->migration_state.notify = virtio_net_migration_state_notifier; add_migration_state_change_notifier(&n->migration_state); diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index c8da637d40..ca68be759f 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -207,7 +207,8 @@ struct VirtIONet { DeviceState *primary_dev; char *primary_device_id; char *standby_id; - bool primary_should_be_hidden; + /* primary failover device is hidden*/ + bool failover_primary_hidden; bool failover; DeviceListener primary_listener; Notifier migration_state; From 518eda9fda49da910d47f5baf66a1c0d1d30cebd Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:30 +0100 Subject: [PATCH 24/65] failover: g_strcmp0() knows how to handle NULL Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-10-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index c221671852..e334f05352 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -818,7 +818,7 @@ static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) int ret = 0; const char *standby_id = qemu_opt_get(opts, "failover_pair_id"); - if (standby_id != NULL && (g_strcmp0(standby_id, n->netclient_name) == 0)) { + if (g_strcmp0(standby_id, n->netclient_name) == 0) { n->primary_device_id = g_strdup(opts->id); ret = 1; } From 19e49bc2e984bd065719fc3595f35368b3ae87cd Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:31 +0100 Subject: [PATCH 25/65] failover: Remove primary_device_opts It was really only used once, in failover_add_primary(). Just search for it on global opts when it is needed. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-11-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 21 +++++---------------- include/hw/virtio/virtio-net.h | 1 - 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index e334f05352..2a99b0e0f6 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -791,17 +791,17 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) static void failover_add_primary(VirtIONet *n, Error **errp) { Error *err = NULL; + QemuOpts *opts; if (n->primary_dev) { return; } - n->primary_device_opts = qemu_opts_find(qemu_find_opts("device"), - n->primary_device_id); - if (n->primary_device_opts) { - n->primary_dev = qdev_device_add(n->primary_device_opts, &err); + opts = qemu_opts_find(qemu_find_opts("device"), n->primary_device_id); + if (opts) { + n->primary_dev = qdev_device_add(opts, &err); if (err) { - qemu_opts_del(n->primary_device_opts); + qemu_opts_del(opts); } } else { error_setg(errp, "Primary device not found"); @@ -856,7 +856,6 @@ static DeviceState *virtio_connect_failover_devices(VirtIONet *n, Error **errp) prim_dev = virtio_net_find_primary(n, &err); if (prim_dev) { n->primary_device_id = g_strdup(prim_dev->id); - n->primary_device_opts = prim_dev->opts; } else { error_propagate(errp, err); } @@ -3113,14 +3112,6 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp) if (!pdev->partially_hotplugged) { return true; } - if (!n->primary_device_opts) { - n->primary_device_opts = qemu_opts_from_qdict(qemu_find_opts("device"), - n->primary_device_dict, - errp); - if (!n->primary_device_opts) { - return false; - } - } primary_bus = n->primary_dev->parent_bus; if (!primary_bus) { error_setg(errp, "virtio_net: couldn't find primary bus"); @@ -3211,8 +3202,6 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, goto out; } - n->primary_device_opts = device_opts; - /* failover_primary_hidden is set during feature negotiation */ hide = qatomic_read(&n->failover_primary_hidden); diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index ca68be759f..7159e6c0a0 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -202,7 +202,6 @@ struct VirtIONet { AnnounceTimer announce_timer; bool needs_vnet_hdr_swap; bool mtu_bypass_backend; - QemuOpts *primary_device_opts; QDict *primary_device_dict; DeviceState *primary_dev; char *primary_device_id; From 4f0303aed87f83715055e558176046a8a3d9b987 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:32 +0100 Subject: [PATCH 26/65] failover: remove standby_id variable We can calculate it, and we only use it once anyways. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-12-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 11 +++-------- include/hw/virtio/virtio-net.h | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 2a99b0e0f6..953d5c2bc8 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3181,23 +3181,19 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, VirtIONet *n = container_of(listener, VirtIONet, primary_listener); bool match_found = false; bool hide = false; + const char *standby_id; if (!device_opts) { return -1; } n->primary_device_dict = qemu_opts_to_qdict(device_opts, n->primary_device_dict); - if (n->primary_device_dict) { - g_free(n->standby_id); - n->standby_id = g_strdup(qdict_get_try_str(n->primary_device_dict, - "failover_pair_id")); - } - if (g_strcmp0(n->standby_id, n->netclient_name) == 0) { + standby_id = qemu_opt_get(device_opts, "failover_pair_id"); + if (g_strcmp0(standby_id, n->netclient_name) == 0) { match_found = true; } else { match_found = false; hide = false; - g_free(n->standby_id); n->primary_device_dict = NULL; goto out; } @@ -3400,7 +3396,6 @@ static void virtio_net_device_unrealize(DeviceState *dev) if (n->failover) { device_listener_unregister(&n->primary_listener); g_free(n->primary_device_id); - g_free(n->standby_id); qobject_unref(n->primary_device_dict); n->primary_device_dict = NULL; } diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 7159e6c0a0..a055f39dd6 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -205,7 +205,6 @@ struct VirtIONet { QDict *primary_device_dict; DeviceState *primary_dev; char *primary_device_id; - char *standby_id; /* primary failover device is hidden*/ bool failover_primary_hidden; bool failover; From 9673a88e97d1eb428872bd261dbf56a0f3c2fd71 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:33 +0100 Subject: [PATCH 27/65] failover: Remove primary_device_dict It was only used once. And we have there opts->id, so no need for it. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-13-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 17 ++++------------- include/hw/virtio/virtio-net.h | 1 - 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 953d5c2bc8..6e5a56a230 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3186,28 +3186,21 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, if (!device_opts) { return -1; } - n->primary_device_dict = qemu_opts_to_qdict(device_opts, - n->primary_device_dict); standby_id = qemu_opt_get(device_opts, "failover_pair_id"); if (g_strcmp0(standby_id, n->netclient_name) == 0) { match_found = true; } else { match_found = false; hide = false; - n->primary_device_dict = NULL; goto out; } /* failover_primary_hidden is set during feature negotiation */ hide = qatomic_read(&n->failover_primary_hidden); - - if (n->primary_device_dict) { - g_free(n->primary_device_id); - n->primary_device_id = g_strdup(qdict_get_try_str( - n->primary_device_dict, "id")); - if (!n->primary_device_id) { - warn_report("primary_device_id not set"); - } + g_free(n->primary_device_id); + n->primary_device_id = g_strdup(device_opts->id); + if (!n->primary_device_id) { + warn_report("primary_device_id not set"); } out: @@ -3396,8 +3389,6 @@ static void virtio_net_device_unrealize(DeviceState *dev) if (n->failover) { device_listener_unregister(&n->primary_listener); g_free(n->primary_device_id); - qobject_unref(n->primary_device_dict); - n->primary_device_dict = NULL; } max_queues = n->multiqueue ? n->max_queues : 1; diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index a055f39dd6..fe353d8299 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -202,7 +202,6 @@ struct VirtIONet { AnnounceTimer announce_timer; bool needs_vnet_hdr_swap; bool mtu_bypass_backend; - QDict *primary_device_dict; DeviceState *primary_dev; char *primary_device_id; /* primary failover device is hidden*/ From 7b3dc2f8c0b817bbe78ba347130b3c99fe2c4470 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:34 +0100 Subject: [PATCH 28/65] failover: Remove memory leak Two things, at this point: * n->primary_device_id has to be set, otherwise virtio_net_find_primary don't work. So we have a leak here. * it has to be exactly the same that prim_dev->id because what qdev_find_recursive() does is just compare this two values. So remove the unneeded assignment and leaky bits. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-14-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 6e5a56a230..70fa372c08 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -854,9 +854,7 @@ static DeviceState *virtio_connect_failover_devices(VirtIONet *n, Error **errp) Error *err = NULL; prim_dev = virtio_net_find_primary(n, &err); - if (prim_dev) { - n->primary_device_id = g_strdup(prim_dev->id); - } else { + if (!prim_dev) { error_propagate(errp, err); } From 7cf05b7ed8e84e89b873701e3dfcd56aa81b2d13 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:35 +0100 Subject: [PATCH 29/65] failover: simplify virtio_net_find_primary() a - is_my_primary() never sets one error b - If we return 1, primary_device_id is always set Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-15-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 70fa372c08..881907d1bd 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -828,24 +828,12 @@ static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) static DeviceState *virtio_net_find_primary(VirtIONet *n, Error **errp) { - DeviceState *dev = NULL; Error *err = NULL; - if (qemu_opts_foreach(qemu_find_opts("device"), - is_my_primary, n, &err)) { - if (err) { - error_propagate(errp, err); - return NULL; - } - if (n->primary_device_id) { - dev = qdev_find_recursive(sysbus_get_default(), - n->primary_device_id); - } else { - error_setg(errp, "Primary device id not found"); - return NULL; - } + if (!qemu_opts_foreach(qemu_find_opts("device"), is_my_primary, n, &err)) { + return NULL; } - return dev; + return qdev_find_recursive(sysbus_get_default(), n->primary_device_id); } static DeviceState *virtio_connect_failover_devices(VirtIONet *n, Error **errp) From 89631fed27bd76b0292d8b2a78291ea96185c87d Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:36 +0100 Subject: [PATCH 30/65] failover: should_be_hidden() should take a bool We didn't use at all the -1 value, and we don't really care. It was only used for the cases when this is not the device that we are searching for. And in that case we should not hide the device. Once there, simplify virtio-Snet_primary_should_be_hidden. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-16-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/core/qdev.c | 19 +++++-------------- hw/net/virtio-net.c | 27 +++++++-------------------- include/hw/qdev-core.h | 2 +- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 262bca716f..8f4b8f3cc1 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -214,26 +214,17 @@ void device_listener_unregister(DeviceListener *listener) bool qdev_should_hide_device(QemuOpts *opts) { - int rc = -1; DeviceListener *listener; QTAILQ_FOREACH(listener, &device_listeners, link) { - if (listener->should_be_hidden) { - /* - * should_be_hidden_will return - * 1 if device matches opts and it should be hidden - * 0 if device matches opts and should not be hidden - * -1 if device doesn't match ops - */ - rc = listener->should_be_hidden(listener, opts); - } - - if (rc > 0) { - break; + if (listener->should_be_hidden) { + if (listener->should_be_hidden(listener, opts)) { + return true; + } } } - return rc > 0; + return false; } void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 881907d1bd..9f12d33da0 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3161,24 +3161,19 @@ static void virtio_net_migration_state_notifier(Notifier *notifier, void *data) virtio_net_handle_migration_primary(n, s); } -static int virtio_net_primary_should_be_hidden(DeviceListener *listener, - QemuOpts *device_opts) +static bool virtio_net_primary_should_be_hidden(DeviceListener *listener, + QemuOpts *device_opts) { VirtIONet *n = container_of(listener, VirtIONet, primary_listener); - bool match_found = false; - bool hide = false; + bool hide; const char *standby_id; if (!device_opts) { - return -1; + return false; } standby_id = qemu_opt_get(device_opts, "failover_pair_id"); - if (g_strcmp0(standby_id, n->netclient_name) == 0) { - match_found = true; - } else { - match_found = false; - hide = false; - goto out; + if (g_strcmp0(standby_id, n->netclient_name) != 0) { + return false; } /* failover_primary_hidden is set during feature negotiation */ @@ -3188,15 +3183,7 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener, if (!n->primary_device_id) { warn_report("primary_device_id not set"); } - -out: - if (match_found && hide) { - return 1; - } else if (match_found && !hide) { - return 0; - } else { - return -1; - } + return hide; } static void virtio_net_device_realize(DeviceState *dev, Error **errp) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 5e737195b5..250f4edef6 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -200,7 +200,7 @@ struct DeviceListener { * inform qdev that a device should be hidden, depending on the device * opts, for example, to hide a standby device. */ - int (*should_be_hidden)(DeviceListener *listener, QemuOpts *device_opts); + bool (*should_be_hidden)(DeviceListener *listener, QemuOpts *device_opts); QTAILQ_ENTRY(DeviceListener) link; }; From b91ad981b867e15171234efc3f2ab4074d377cef Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:37 +0100 Subject: [PATCH 31/65] failover: Rename function to hide_device() You should not use pasive. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-17-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/core/qdev.c | 4 ++-- hw/net/virtio-net.c | 7 +++---- include/hw/qdev-core.h | 28 +++++++++++++++------------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 8f4b8f3cc1..cbdff0b6c6 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -217,8 +217,8 @@ bool qdev_should_hide_device(QemuOpts *opts) DeviceListener *listener; QTAILQ_FOREACH(listener, &device_listeners, link) { - if (listener->should_be_hidden) { - if (listener->should_be_hidden(listener, opts)) { + if (listener->hide_device) { + if (listener->hide_device(listener, opts)) { return true; } } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 9f12d33da0..747614ff2a 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3161,8 +3161,8 @@ static void virtio_net_migration_state_notifier(Notifier *notifier, void *data) virtio_net_handle_migration_primary(n, s); } -static bool virtio_net_primary_should_be_hidden(DeviceListener *listener, - QemuOpts *device_opts) +static bool failover_hide_primary_device(DeviceListener *listener, + QemuOpts *device_opts) { VirtIONet *n = container_of(listener, VirtIONet, primary_listener); bool hide; @@ -3220,8 +3220,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) } if (n->failover) { - n->primary_listener.should_be_hidden = - virtio_net_primary_should_be_hidden; + n->primary_listener.hide_device = failover_hide_primary_device; qatomic_set(&n->failover_primary_hidden, true); device_listener_register(&n->primary_listener); n->migration_state.notify = virtio_net_migration_state_notifier; diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 250f4edef6..6ac86db44e 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -81,16 +81,17 @@ typedef void (*BusUnrealize)(BusState *bus); * * * # Hiding a device # - * To hide a device, a DeviceListener function should_be_hidden() needs to + * To hide a device, a DeviceListener function hide_device() needs to * be registered. - * It can be used to defer adding a device and therefore hide it from the - * guest. The handler registering to this DeviceListener can save the QOpts - * passed to it for re-using it later and must return that it wants the device - * to be/remain hidden or not. When the handler function decides the device - * shall not be hidden it will be added in qdev_device_add() and - * realized as any other device. Otherwise qdev_device_add() will return early - * without adding the device. The guest will not see a "hidden" device - * until it was marked don't hide and qdev_device_add called again. + * It can be used to defer adding a device and therefore hide it from + * the guest. The handler registering to this DeviceListener can save + * the QOpts passed to it for re-using it later. It must return if it + * wants the device to be hidden or visible. When the handler function + * decides the device shall be visible it will be added with + * qdev_device_add() and realized as any other device. Otherwise + * qdev_device_add() will return early without adding the device. The + * guest will not see a "hidden" device until it was marked visible + * and qdev_device_add called again. * */ struct DeviceClass { @@ -196,11 +197,12 @@ struct DeviceListener { void (*realize)(DeviceListener *listener, DeviceState *dev); void (*unrealize)(DeviceListener *listener, DeviceState *dev); /* - * This callback is called upon init of the DeviceState and allows to - * inform qdev that a device should be hidden, depending on the device - * opts, for example, to hide a standby device. + * This callback is called upon init of the DeviceState and + * informs qdev if a device should be visible or hidden. We can + * hide a failover device depending for example on the device + * opts. */ - bool (*should_be_hidden)(DeviceListener *listener, QemuOpts *device_opts); + bool (*hide_device)(DeviceListener *listener, QemuOpts *device_opts); QTAILQ_ENTRY(DeviceListener) link; }; From 0763db4f2df3a92336d78e8b68a665f7d1a1bc66 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:38 +0100 Subject: [PATCH 32/65] failover: virtio_net_connect_failover_devices() does nothing It just calls virtio_net_find_primary(), so just update the callers. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-18-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 747614ff2a..c6200b924e 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -836,19 +836,6 @@ static DeviceState *virtio_net_find_primary(VirtIONet *n, Error **errp) return qdev_find_recursive(sysbus_get_default(), n->primary_device_id); } -static DeviceState *virtio_connect_failover_devices(VirtIONet *n, Error **errp) -{ - DeviceState *prim_dev = NULL; - Error *err = NULL; - - prim_dev = virtio_net_find_primary(n, &err); - if (!prim_dev) { - error_propagate(errp, err); - } - - return prim_dev; -} - static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) { VirtIONet *n = VIRTIO_NET(vdev); @@ -904,7 +891,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) qatomic_set(&n->failover_primary_hidden, false); failover_add_primary(n, &err); if (err) { - n->primary_dev = virtio_connect_failover_devices(n, &err); + n->primary_dev = virtio_net_find_primary(n, &err); if (err) { goto out_err; } @@ -3128,7 +3115,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, should_be_hidden = qatomic_read(&n->failover_primary_hidden); if (!n->primary_dev) { - n->primary_dev = virtio_connect_failover_devices(n, &err); + n->primary_dev = virtio_net_find_primary(n, &err); if (!n->primary_dev) { return; } From 85d3b93196e43c4493c118aa9e3a82fe657636b5 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:39 +0100 Subject: [PATCH 33/65] failover: Rename to failover_find_primary_device() This commit: * Rename them to failover_find_primary_devices() so - it starts with failover_ - it don't connect anything, just find the primary device * Create documentation for the function Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-19-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index c6200b924e..ff82f1017d 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -826,7 +826,13 @@ static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) return ret; } -static DeviceState *virtio_net_find_primary(VirtIONet *n, Error **errp) +/** + * Find the primary device for this failover virtio-net + * + * @n: VirtIONet device + * @errp: returns an error if this function fails + */ +static DeviceState *failover_find_primary_device(VirtIONet *n, Error **errp) { Error *err = NULL; @@ -891,7 +897,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) qatomic_set(&n->failover_primary_hidden, false); failover_add_primary(n, &err); if (err) { - n->primary_dev = virtio_net_find_primary(n, &err); + n->primary_dev = failover_find_primary_device(n, &err); if (err) { goto out_err; } @@ -3115,7 +3121,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, should_be_hidden = qatomic_read(&n->failover_primary_hidden); if (!n->primary_dev) { - n->primary_dev = virtio_net_find_primary(n, &err); + n->primary_dev = failover_find_primary_device(n, &err); if (!n->primary_dev) { return; } From 5f2ef3b0d032797b6bad9449dfece3a8111a8529 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:40 +0100 Subject: [PATCH 34/65] failover: simplify qdev_device_add() failover case Just put allthe logic inside the same if. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-20-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- softmmu/qdev-monitor.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index a25f5d612c..12b7540f17 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -600,7 +600,6 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) const char *driver, *path; DeviceState *dev = NULL; BusState *bus = NULL; - bool hide; driver = qemu_opt_get(opts, "driver"); if (!driver) { @@ -634,14 +633,16 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) return NULL; } } - hide = should_hide_device(opts); - if ((hide || qdev_hotplug) && bus && !qbus_is_hotpluggable(bus)) { - error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); + if (should_hide_device(opts)) { + if (bus && !qbus_is_hotpluggable(bus)) { + error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); + } return NULL; } - if (hide) { + if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) { + error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); return NULL; } From 2e28095369f4eab516852fd49dde17c3bfd782f9 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:41 +0100 Subject: [PATCH 35/65] failover: simplify qdev_device_add() We don't need to walk the opts by hand. qmp_opt_get() already does that. And then we can remove the functions that did that walk. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-21-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- softmmu/qdev-monitor.c | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 12b7540f17..0e10f0466f 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -572,28 +572,6 @@ void qdev_set_id(DeviceState *dev, const char *id) } } -static int is_failover_device(void *opaque, const char *name, const char *value, - Error **errp) -{ - if (strcmp(name, "failover_pair_id") == 0) { - QemuOpts *opts = opaque; - - if (qdev_should_hide_device(opts)) { - return 1; - } - } - - return 0; -} - -static bool should_hide_device(QemuOpts *opts) -{ - if (qemu_opt_foreach(opts, is_failover_device, opts, NULL) == 0) { - return false; - } - return true; -} - DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) { DeviceClass *dc; @@ -634,11 +612,13 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) } } - if (should_hide_device(opts)) { - if (bus && !qbus_is_hotpluggable(bus)) { - error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); + if (qemu_opt_get(opts, "failover_pair_id")) { + if (qdev_should_hide_device(opts)) { + if (bus && !qbus_is_hotpluggable(bus)) { + error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); + } + return NULL; } - return NULL; } if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) { From fec037c1e2da0a7ea54eabce65cc14d461fdc5eb Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:42 +0100 Subject: [PATCH 36/65] failover: make sure that id always exist We check that it exist at device creation time, so we don't have to check anywhere else. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-22-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 3 --- softmmu/qdev-monitor.c | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index ff82f1017d..c708c03cf6 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3173,9 +3173,6 @@ static bool failover_hide_primary_device(DeviceListener *listener, hide = qatomic_read(&n->failover_primary_hidden); g_free(n->primary_device_id); n->primary_device_id = g_strdup(device_opts->id); - if (!n->primary_device_id) { - warn_report("primary_device_id not set"); - } return hide; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 0e10f0466f..301089eaea 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -613,6 +613,10 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) } if (qemu_opt_get(opts, "failover_pair_id")) { + if (!opts->id) { + error_setg(errp, "Device with failover_pair_id don't have id"); + return NULL; + } if (qdev_should_hide_device(opts)) { if (bus && !qbus_is_hotpluggable(bus)) { error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); From 0a0a27d66bcb275e5b984d8758880a7eff75464e Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:43 +0100 Subject: [PATCH 37/65] failover: remove failover_find_primary_device() error parameter It can never give one error. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-23-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index c708c03cf6..b994796734 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -832,7 +832,7 @@ static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) * @n: VirtIONet device * @errp: returns an error if this function fails */ -static DeviceState *failover_find_primary_device(VirtIONet *n, Error **errp) +static DeviceState *failover_find_primary_device(VirtIONet *n) { Error *err = NULL; @@ -897,10 +897,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) qatomic_set(&n->failover_primary_hidden, false); failover_add_primary(n, &err); if (err) { - n->primary_dev = failover_find_primary_device(n, &err); - if (err) { - goto out_err; - } + n->primary_dev = failover_find_primary_device(n); failover_add_primary(n, &err); if (err) { goto out_err; @@ -3121,7 +3118,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, should_be_hidden = qatomic_read(&n->failover_primary_hidden); if (!n->primary_dev) { - n->primary_dev = failover_find_primary_device(n, &err); + n->primary_dev = failover_find_primary_device(n); if (!n->primary_dev) { return; } From f5e1847ba50a8d1adf66c0cf312e53c162e52487 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:44 +0100 Subject: [PATCH 38/65] failover: split failover_find_primary_device_id() So we can calculate the device id when we need it. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-24-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 63 +++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index b994796734..2c502c13fd 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -788,6 +788,49 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) return virtio_net_guest_offloads_by_features(vdev->guest_features); } +typedef struct { + VirtIONet *n; + char *id; +} FailoverId; + +/** + * Set the id of the failover primary device + * + * @opaque: FailoverId to setup + * @opts: opts for device we are handling + * @errp: returns an error if this function fails + */ +static int failover_set_primary(void *opaque, QemuOpts *opts, Error **errp) +{ + FailoverId *fid = opaque; + const char *standby_id = qemu_opt_get(opts, "failover_pair_id"); + + if (g_strcmp0(standby_id, fid->n->netclient_name) == 0) { + fid->id = g_strdup(opts->id); + return 1; + } + + return 0; +} + +/** + * Find the primary device id for this failover virtio-net + * + * @n: VirtIONet device + * @errp: returns an error if this function fails + */ +static char *failover_find_primary_device_id(VirtIONet *n) +{ + Error *err = NULL; + FailoverId fid; + + if (!qemu_opts_foreach(qemu_find_opts("device"), + failover_set_primary, &fid, &err)) { + return NULL; + } + return fid.id; +} + static void failover_add_primary(VirtIONet *n, Error **errp) { Error *err = NULL; @@ -812,20 +855,6 @@ static void failover_add_primary(VirtIONet *n, Error **errp) error_propagate(errp, err); } -static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) -{ - VirtIONet *n = opaque; - int ret = 0; - const char *standby_id = qemu_opt_get(opts, "failover_pair_id"); - - if (g_strcmp0(standby_id, n->netclient_name) == 0) { - n->primary_device_id = g_strdup(opts->id); - ret = 1; - } - - return ret; -} - /** * Find the primary device for this failover virtio-net * @@ -834,11 +863,13 @@ static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp) */ static DeviceState *failover_find_primary_device(VirtIONet *n) { - Error *err = NULL; + char *id = failover_find_primary_device_id(n); - if (!qemu_opts_foreach(qemu_find_opts("device"), is_my_primary, n, &err)) { + if (!id) { return NULL; } + n->primary_device_id = g_strdup(id); + return qdev_find_recursive(sysbus_get_default(), n->primary_device_id); } From 3abad4a221e050d43fa8540677b285057642baaf Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:45 +0100 Subject: [PATCH 39/65] failover: We don't need to cache primary_device_id anymore Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-25-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 20 ++++++++++---------- include/hw/virtio/virtio-net.h | 1 - 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 2c502c13fd..746ed3fb71 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -824,6 +824,7 @@ static char *failover_find_primary_device_id(VirtIONet *n) Error *err = NULL; FailoverId fid; + fid.n = n; if (!qemu_opts_foreach(qemu_find_opts("device"), failover_set_primary, &fid, &err)) { return NULL; @@ -835,12 +836,17 @@ static void failover_add_primary(VirtIONet *n, Error **errp) { Error *err = NULL; QemuOpts *opts; + char *id; if (n->primary_dev) { return; } - opts = qemu_opts_find(qemu_find_opts("device"), n->primary_device_id); + id = failover_find_primary_device_id(n); + if (!id) { + return; + } + opts = qemu_opts_find(qemu_find_opts("device"), id); if (opts) { n->primary_dev = qdev_device_add(opts, &err); if (err) { @@ -868,9 +874,8 @@ static DeviceState *failover_find_primary_device(VirtIONet *n) if (!id) { return NULL; } - n->primary_device_id = g_strdup(id); - return qdev_find_recursive(sysbus_get_default(), n->primary_device_id); + return qdev_find_recursive(sysbus_get_default(), id); } static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) @@ -3160,7 +3165,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, vmstate_unregister(VMSTATE_IF(n->primary_dev), qdev_get_vmsd(n->primary_dev), n->primary_dev); - qapi_event_send_unplug_primary(n->primary_device_id); + qapi_event_send_unplug_primary(n->primary_dev->id); qatomic_set(&n->failover_primary_hidden, true); } else { warn_report("couldn't unplug primary device"); @@ -3186,7 +3191,6 @@ static bool failover_hide_primary_device(DeviceListener *listener, QemuOpts *device_opts) { VirtIONet *n = container_of(listener, VirtIONet, primary_listener); - bool hide; const char *standby_id; if (!device_opts) { @@ -3198,10 +3202,7 @@ static bool failover_hide_primary_device(DeviceListener *listener, } /* failover_primary_hidden is set during feature negotiation */ - hide = qatomic_read(&n->failover_primary_hidden); - g_free(n->primary_device_id); - n->primary_device_id = g_strdup(device_opts->id); - return hide; + return qatomic_read(&n->failover_primary_hidden); } static void virtio_net_device_realize(DeviceState *dev, Error **errp) @@ -3378,7 +3379,6 @@ static void virtio_net_device_unrealize(DeviceState *dev) if (n->failover) { device_listener_unregister(&n->primary_listener); - g_free(n->primary_device_id); } max_queues = n->multiqueue ? n->max_queues : 1; diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index fe353d8299..efef64e02f 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -203,7 +203,6 @@ struct VirtIONet { bool needs_vnet_hdr_swap; bool mtu_bypass_backend; DeviceState *primary_dev; - char *primary_device_id; /* primary failover device is hidden*/ bool failover_primary_hidden; bool failover; From 0e9a65c5b168b993b845ec2acb2568328c2353da Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:46 +0100 Subject: [PATCH 40/65] failover: Caller of this two functions already have primary_dev Pass it as an argument. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-26-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 746ed3fb71..b37e9cd1d9 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3093,17 +3093,17 @@ void virtio_net_set_netclient_name(VirtIONet *n, const char *name, n->netclient_type = g_strdup(type); } -static bool failover_unplug_primary(VirtIONet *n) +static bool failover_unplug_primary(VirtIONet *n, DeviceState *dev) { HotplugHandler *hotplug_ctrl; PCIDevice *pci_dev; Error *err = NULL; - hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev); + hotplug_ctrl = qdev_get_hotplug_handler(dev); if (hotplug_ctrl) { - pci_dev = PCI_DEVICE(n->primary_dev); + pci_dev = PCI_DEVICE(dev); pci_dev->partially_hotplugged = true; - hotplug_handler_unplug_request(hotplug_ctrl, n->primary_dev, &err); + hotplug_handler_unplug_request(hotplug_ctrl, dev, &err); if (err) { error_report_err(err); return false; @@ -3114,30 +3114,31 @@ static bool failover_unplug_primary(VirtIONet *n) return true; } -static bool failover_replug_primary(VirtIONet *n, Error **errp) +static bool failover_replug_primary(VirtIONet *n, DeviceState *dev, + Error **errp) { Error *err = NULL; HotplugHandler *hotplug_ctrl; - PCIDevice *pdev = PCI_DEVICE(n->primary_dev); + PCIDevice *pdev = PCI_DEVICE(dev); BusState *primary_bus; if (!pdev->partially_hotplugged) { return true; } - primary_bus = n->primary_dev->parent_bus; + primary_bus = dev->parent_bus; if (!primary_bus) { error_setg(errp, "virtio_net: couldn't find primary bus"); return false; } - qdev_set_parent_bus(n->primary_dev, primary_bus, &error_abort); + qdev_set_parent_bus(dev, primary_bus, &error_abort); qatomic_set(&n->failover_primary_hidden, false); - hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev); + hotplug_ctrl = qdev_get_hotplug_handler(dev); if (hotplug_ctrl) { - hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, &err); + hotplug_handler_pre_plug(hotplug_ctrl, dev, &err); if (err) { goto out; } - hotplug_handler_plug(hotplug_ctrl, n->primary_dev, &err); + hotplug_handler_plug(hotplug_ctrl, dev, &err); } out: @@ -3161,7 +3162,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, } if (migration_in_setup(s) && !should_be_hidden) { - if (failover_unplug_primary(n)) { + if (failover_unplug_primary(n, n->primary_dev)) { vmstate_unregister(VMSTATE_IF(n->primary_dev), qdev_get_vmsd(n->primary_dev), n->primary_dev); @@ -3172,7 +3173,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, } } else if (migration_has_failed(s)) { /* We already unplugged the device let's plug it back */ - if (!failover_replug_primary(n, &err)) { + if (!failover_replug_primary(n, n->primary_dev, &err)) { if (err) { error_report_err(err); } From 07a5d816d50f5f876d5fcd43724a6ff17cf59a4f Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:47 +0100 Subject: [PATCH 41/65] failover: simplify failover_unplug_primary We can calculate device just once. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-27-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index b37e9cd1d9..9203d81780 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3146,34 +3146,29 @@ out: return !err; } -static void virtio_net_handle_migration_primary(VirtIONet *n, - MigrationState *s) +static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s) { bool should_be_hidden; Error *err = NULL; + DeviceState *dev = failover_find_primary_device(n); + + if (!dev) { + return; + } should_be_hidden = qatomic_read(&n->failover_primary_hidden); - if (!n->primary_dev) { - n->primary_dev = failover_find_primary_device(n); - if (!n->primary_dev) { - return; - } - } - if (migration_in_setup(s) && !should_be_hidden) { - if (failover_unplug_primary(n, n->primary_dev)) { - vmstate_unregister(VMSTATE_IF(n->primary_dev), - qdev_get_vmsd(n->primary_dev), - n->primary_dev); - qapi_event_send_unplug_primary(n->primary_dev->id); + if (failover_unplug_primary(n, dev)) { + vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev); + qapi_event_send_unplug_primary(dev->id); qatomic_set(&n->failover_primary_hidden, true); } else { warn_report("couldn't unplug primary device"); } } else if (migration_has_failed(s)) { /* We already unplugged the device let's plug it back */ - if (!failover_replug_primary(n, n->primary_dev, &err)) { + if (!failover_replug_primary(n, dev, &err)) { if (err) { error_report_err(err); } From 21e8709b29cd981c74565e75276ed476c954cbbf Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 18 Nov 2020 09:37:48 +0100 Subject: [PATCH 42/65] failover: Remove primary_dev member Only three uses remained, and we can remove them on that case. Signed-off-by: Juan Quintela Message-Id: <20201118083748.1328-28-quintela@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 75 +++++++++++++++------------------- include/hw/virtio/virtio-net.h | 1 - 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 9203d81780..044ac95f6f 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -832,35 +832,6 @@ static char *failover_find_primary_device_id(VirtIONet *n) return fid.id; } -static void failover_add_primary(VirtIONet *n, Error **errp) -{ - Error *err = NULL; - QemuOpts *opts; - char *id; - - if (n->primary_dev) { - return; - } - - id = failover_find_primary_device_id(n); - if (!id) { - return; - } - opts = qemu_opts_find(qemu_find_opts("device"), id); - if (opts) { - n->primary_dev = qdev_device_add(opts, &err); - if (err) { - qemu_opts_del(opts); - } - } else { - error_setg(errp, "Primary device not found"); - error_append_hint(errp, "Virtio-net failover will not work. Make " - "sure primary device has parameter" - " failover_pair_id=\n"); - } - error_propagate(errp, err); -} - /** * Find the primary device for this failover virtio-net * @@ -878,6 +849,36 @@ static DeviceState *failover_find_primary_device(VirtIONet *n) return qdev_find_recursive(sysbus_get_default(), id); } +static void failover_add_primary(VirtIONet *n, Error **errp) +{ + Error *err = NULL; + QemuOpts *opts; + char *id; + DeviceState *dev = failover_find_primary_device(n); + + if (dev) { + return; + } + + id = failover_find_primary_device_id(n); + if (!id) { + return; + } + opts = qemu_opts_find(qemu_find_opts("device"), id); + if (opts) { + dev = qdev_device_add(opts, &err); + if (err) { + qemu_opts_del(opts); + } + } else { + error_setg(errp, "Primary device not found"); + error_append_hint(errp, "Virtio-net failover will not work. Make " + "sure primary device has parameter" + " failover_pair_id=\n"); + } + error_propagate(errp, err); +} + static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) { VirtIONet *n = VIRTIO_NET(vdev); @@ -933,19 +934,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) qatomic_set(&n->failover_primary_hidden, false); failover_add_primary(n, &err); if (err) { - n->primary_dev = failover_find_primary_device(n); - failover_add_primary(n, &err); - if (err) { - goto out_err; - } + warn_report_err(err); } } - return; - -out_err: - if (err) { - warn_report_err(err); - } } static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd, @@ -3420,13 +3411,15 @@ static int virtio_net_pre_save(void *opaque) static bool primary_unplug_pending(void *opaque) { DeviceState *dev = opaque; + DeviceState *primary; VirtIODevice *vdev = VIRTIO_DEVICE(dev); VirtIONet *n = VIRTIO_NET(vdev); if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_STANDBY)) { return false; } - return n->primary_dev ? n->primary_dev->pending_deleted_event : false; + primary = failover_find_primary_device(n); + return primary ? primary->pending_deleted_event : false; } static bool dev_unplug_pending(void *opaque) diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index efef64e02f..7e96d193aa 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -202,7 +202,6 @@ struct VirtIONet { AnnounceTimer announce_timer; bool needs_vnet_hdr_swap; bool mtu_bypass_backend; - DeviceState *primary_dev; /* primary failover device is hidden*/ bool failover_primary_hidden; bool failover; From 576a00bdeb5bf93275e1d8e923d71daa80935f21 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Mon, 9 Nov 2020 18:39:28 +0100 Subject: [PATCH 43/65] hw: add compat machines for 6.0 Add 6.0 machine types for arm/i440fx/q35/s390x/spapr. Signed-off-by: Cornelia Huck Message-Id: <20201109173928.1001764-1-cohuck@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/virt.c | 11 +++++++++-- hw/core/machine.c | 3 +++ hw/i386/pc.c | 3 +++ hw/i386/pc_piix.c | 14 +++++++++++++- hw/i386/pc_q35.c | 13 ++++++++++++- hw/ppc/spapr.c | 17 ++++++++++++++--- hw/s390x/s390-virtio-ccw.c | 14 +++++++++++++- include/hw/boards.h | 3 +++ include/hw/i386/pc.h | 3 +++ 9 files changed, 73 insertions(+), 8 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 847257aa5c..22572c32b7 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2590,10 +2590,17 @@ static void machvirt_machine_init(void) } type_init(machvirt_machine_init); -static void virt_machine_5_2_options(MachineClass *mc) +static void virt_machine_6_0_options(MachineClass *mc) { } -DEFINE_VIRT_MACHINE_AS_LATEST(5, 2) +DEFINE_VIRT_MACHINE_AS_LATEST(6, 0) + +static void virt_machine_5_2_options(MachineClass *mc) +{ + virt_machine_6_0_options(mc); + compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); +} +DEFINE_VIRT_MACHINE(5, 2) static void virt_machine_5_1_options(MachineClass *mc) { diff --git a/hw/core/machine.c b/hw/core/machine.c index d0408049b5..9c41b94e0d 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -28,6 +28,9 @@ #include "hw/mem/nvdimm.h" #include "migration/vmstate.h" +GlobalProperty hw_compat_5_2[] = {}; +const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); + GlobalProperty hw_compat_5_1[] = { { "vhost-scsi", "num_queues", "1"}, { "vhost-user-blk", "num-queues", "1"}, diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 76a846ff9a..7113fb0770 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -97,6 +97,9 @@ #include "trace.h" #include CONFIG_DEVICES +GlobalProperty pc_compat_5_2[] = {}; +const size_t pc_compat_5_2_len = G_N_ELEMENTS(pc_compat_5_2); + GlobalProperty pc_compat_5_1[] = { { "ICH9-LPC", "x-smi-cpu-hotplug", "off" }, }; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 13d1628f13..6188c3e97e 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -426,7 +426,7 @@ static void pc_i440fx_machine_options(MachineClass *m) machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE); } -static void pc_i440fx_5_2_machine_options(MachineClass *m) +static void pc_i440fx_6_0_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_i440fx_machine_options(m); @@ -435,6 +435,18 @@ static void pc_i440fx_5_2_machine_options(MachineClass *m) pcmc->default_cpu_version = 1; } +DEFINE_I440FX_MACHINE(v6_0, "pc-i440fx-6.0", NULL, + pc_i440fx_6_0_machine_options); + +static void pc_i440fx_5_2_machine_options(MachineClass *m) +{ + pc_i440fx_6_0_machine_options(m); + m->alias = NULL; + m->is_default = false; + compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len); + compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len); +} + DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL, pc_i440fx_5_2_machine_options); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index a3f4959c43..0a212443aa 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -344,7 +344,7 @@ static void pc_q35_machine_options(MachineClass *m) m->max_cpus = 288; } -static void pc_q35_5_2_machine_options(MachineClass *m) +static void pc_q35_6_0_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_q35_machine_options(m); @@ -352,6 +352,17 @@ static void pc_q35_5_2_machine_options(MachineClass *m) pcmc->default_cpu_version = 1; } +DEFINE_Q35_MACHINE(v6_0, "pc-q35-6.0", NULL, + pc_q35_6_0_machine_options); + +static void pc_q35_5_2_machine_options(MachineClass *m) +{ + pc_q35_6_0_machine_options(m); + m->alias = NULL; + compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len); + compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len); +} + DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL, pc_q35_5_2_machine_options); diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 12a012d9dd..c060702013 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4510,14 +4510,25 @@ static void spapr_machine_latest_class_options(MachineClass *mc) type_init(spapr_machine_register_##suffix) /* - * pseries-5.2 + * pseries-6.0 */ -static void spapr_machine_5_2_class_options(MachineClass *mc) +static void spapr_machine_6_0_class_options(MachineClass *mc) { /* Defaults for the latest behaviour inherited from the base class */ } -DEFINE_SPAPR_MACHINE(5_2, "5.2", true); +DEFINE_SPAPR_MACHINE(6_0, "6.0", true); + +/* + * pseries-5.2 + */ +static void spapr_machine_5_2_class_options(MachineClass *mc) +{ + spapr_machine_6_0_class_options(mc); + compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); +} + +DEFINE_SPAPR_MACHINE(5_2, "5.2", false); /* * pseries-5.1 diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 4e140bbead..f0ee8dae68 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -789,14 +789,26 @@ bool css_migration_enabled(void) } \ type_init(ccw_machine_register_##suffix) +static void ccw_machine_6_0_instance_options(MachineState *machine) +{ +} + +static void ccw_machine_6_0_class_options(MachineClass *mc) +{ +} +DEFINE_CCW_MACHINE(6_0, "6.0", true); + static void ccw_machine_5_2_instance_options(MachineState *machine) { + ccw_machine_6_0_instance_options(machine); } static void ccw_machine_5_2_class_options(MachineClass *mc) { + ccw_machine_6_0_class_options(mc); + compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); } -DEFINE_CCW_MACHINE(5_2, "5.2", true); +DEFINE_CCW_MACHINE(5_2, "5.2", false); static void ccw_machine_5_1_instance_options(MachineState *machine) { diff --git a/include/hw/boards.h b/include/hw/boards.h index a49e3a6b44..f94f4ad5d8 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -310,6 +310,9 @@ struct MachineState { } \ type_init(machine_initfn##_register_types) +extern GlobalProperty hw_compat_5_2[]; +extern const size_t hw_compat_5_2_len; + extern GlobalProperty hw_compat_5_1[]; extern const size_t hw_compat_5_1_len; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 911e460097..49dfa667de 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -191,6 +191,9 @@ void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory); void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, const CPUArchIdList *apic_ids, GArray *entry); +extern GlobalProperty pc_compat_5_2[]; +extern const size_t pc_compat_5_2_len; + extern GlobalProperty pc_compat_5_1[]; extern const size_t pc_compat_5_1_len; From 0ca293155b7bada98afdb250b74577fd5895f0bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:33 +0400 Subject: [PATCH 44/65] libvhost-user: replace qemu/bswap.h with glibc endian.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Stefan Hajnoczi Message-Id: <20201125100640.366523-2-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/libvhost-user/libvhost-user.c | 77 ++++++++++++++------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index 5c73ffdd6b..1c1cfbf1e7 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "qemu/compiler.h" #if defined(__linux__) @@ -42,7 +43,6 @@ #include "qemu/atomic.h" #include "qemu/osdep.h" -#include "qemu/bswap.h" #include "qemu/memfd.h" #include "libvhost-user.h" @@ -1081,7 +1081,7 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg) return false; } - vq->used_idx = lduw_le_p(&vq->vring.used->idx); + vq->used_idx = le16toh(vq->vring.used->idx); if (vq->last_avail_idx != vq->used_idx) { bool resume = dev->iface->queue_is_processed_in_order && @@ -1198,7 +1198,7 @@ vu_check_queue_inflights(VuDev *dev, VuVirtq *vq) return 0; } - vq->used_idx = lduw_le_p(&vq->vring.used->idx); + vq->used_idx = le16toh(vq->vring.used->idx); vq->resubmit_num = 0; vq->resubmit_list = NULL; vq->counter = 0; @@ -2031,13 +2031,13 @@ vu_queue_started(const VuDev *dev, const VuVirtq *vq) static inline uint16_t vring_avail_flags(VuVirtq *vq) { - return lduw_le_p(&vq->vring.avail->flags); + return le16toh(vq->vring.avail->flags); } static inline uint16_t vring_avail_idx(VuVirtq *vq) { - vq->shadow_avail_idx = lduw_le_p(&vq->vring.avail->idx); + vq->shadow_avail_idx = le16toh(vq->vring.avail->idx); return vq->shadow_avail_idx; } @@ -2045,7 +2045,7 @@ vring_avail_idx(VuVirtq *vq) static inline uint16_t vring_avail_ring(VuVirtq *vq, int i) { - return lduw_le_p(&vq->vring.avail->ring[i]); + return le16toh(vq->vring.avail->ring[i]); } static inline uint16_t @@ -2133,12 +2133,12 @@ virtqueue_read_next_desc(VuDev *dev, struct vring_desc *desc, int i, unsigned int max, unsigned int *next) { /* If this descriptor says it doesn't chain, we're done. */ - if (!(lduw_le_p(&desc[i].flags) & VRING_DESC_F_NEXT)) { + if (!(le16toh(desc[i].flags) & VRING_DESC_F_NEXT)) { return VIRTQUEUE_READ_DESC_DONE; } /* Check they're not leading us off end of descriptors. */ - *next = lduw_le_p(&desc[i].next); + *next = le16toh(desc[i].next); /* Make sure compiler knows to grab that: we don't want it changing! */ smp_wmb(); @@ -2181,8 +2181,8 @@ vu_queue_get_avail_bytes(VuDev *dev, VuVirtq *vq, unsigned int *in_bytes, } desc = vq->vring.desc; - if (lduw_le_p(&desc[i].flags) & VRING_DESC_F_INDIRECT) { - if (ldl_le_p(&desc[i].len) % sizeof(struct vring_desc)) { + if (le16toh(desc[i].flags) & VRING_DESC_F_INDIRECT) { + if (le32toh(desc[i].len) % sizeof(struct vring_desc)) { vu_panic(dev, "Invalid size for indirect buffer table"); goto err; } @@ -2195,8 +2195,8 @@ vu_queue_get_avail_bytes(VuDev *dev, VuVirtq *vq, unsigned int *in_bytes, /* loop over the indirect descriptor table */ indirect = 1; - desc_addr = ldq_le_p(&desc[i].addr); - desc_len = ldl_le_p(&desc[i].len); + desc_addr = le64toh(desc[i].addr); + desc_len = le32toh(desc[i].len); max = desc_len / sizeof(struct vring_desc); read_len = desc_len; desc = vu_gpa_to_va(dev, &read_len, desc_addr); @@ -2223,10 +2223,10 @@ vu_queue_get_avail_bytes(VuDev *dev, VuVirtq *vq, unsigned int *in_bytes, goto err; } - if (lduw_le_p(&desc[i].flags) & VRING_DESC_F_WRITE) { - in_total += ldl_le_p(&desc[i].len); + if (le16toh(desc[i].flags) & VRING_DESC_F_WRITE) { + in_total += le32toh(desc[i].len); } else { - out_total += ldl_le_p(&desc[i].len); + out_total += le32toh(desc[i].len); } if (in_total >= max_in_bytes && out_total >= max_out_bytes) { goto done; @@ -2377,7 +2377,7 @@ vring_used_flags_set_bit(VuVirtq *vq, int mask) flags = (uint16_t *)((char*)vq->vring.used + offsetof(struct vring_used, flags)); - stw_le_p(flags, lduw_le_p(flags) | mask); + *flags = htole16(le16toh(*flags) | mask); } static inline void @@ -2387,17 +2387,20 @@ vring_used_flags_unset_bit(VuVirtq *vq, int mask) flags = (uint16_t *)((char*)vq->vring.used + offsetof(struct vring_used, flags)); - stw_le_p(flags, lduw_le_p(flags) & ~mask); + *flags = htole16(le16toh(*flags) & ~mask); } static inline void vring_set_avail_event(VuVirtq *vq, uint16_t val) { + uint16_t *avail; + if (!vq->notification) { return; } - stw_le_p(&vq->vring.used->ring[vq->vring.num], val); + avail = (uint16_t *)&vq->vring.used->ring[vq->vring.num]; + *avail = htole16(val); } void @@ -2487,15 +2490,15 @@ vu_queue_map_desc(VuDev *dev, VuVirtq *vq, unsigned int idx, size_t sz) struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE]; int rc; - if (lduw_le_p(&desc[i].flags) & VRING_DESC_F_INDIRECT) { - if (ldl_le_p(&desc[i].len) % sizeof(struct vring_desc)) { + if (le16toh(desc[i].flags) & VRING_DESC_F_INDIRECT) { + if (le32toh(desc[i].len) % sizeof(struct vring_desc)) { vu_panic(dev, "Invalid size for indirect buffer table"); return NULL; } /* loop over the indirect descriptor table */ - desc_addr = ldq_le_p(&desc[i].addr); - desc_len = ldl_le_p(&desc[i].len); + desc_addr = le64toh(desc[i].addr); + desc_len = le32toh(desc[i].len); max = desc_len / sizeof(struct vring_desc); read_len = desc_len; desc = vu_gpa_to_va(dev, &read_len, desc_addr); @@ -2517,11 +2520,11 @@ vu_queue_map_desc(VuDev *dev, VuVirtq *vq, unsigned int idx, size_t sz) /* Collect all the descriptors */ do { - if (lduw_le_p(&desc[i].flags) & VRING_DESC_F_WRITE) { + if (le16toh(desc[i].flags) & VRING_DESC_F_WRITE) { if (!virtqueue_map_desc(dev, &in_num, iov + out_num, VIRTQUEUE_MAX_SIZE - out_num, true, - ldq_le_p(&desc[i].addr), - ldl_le_p(&desc[i].len))) { + le64toh(desc[i].addr), + le32toh(desc[i].len))) { return NULL; } } else { @@ -2531,8 +2534,8 @@ vu_queue_map_desc(VuDev *dev, VuVirtq *vq, unsigned int idx, size_t sz) } if (!virtqueue_map_desc(dev, &out_num, iov, VIRTQUEUE_MAX_SIZE, false, - ldq_le_p(&desc[i].addr), - ldl_le_p(&desc[i].len))) { + le64toh(desc[i].addr), + le32toh(desc[i].len))) { return NULL; } } @@ -2731,15 +2734,15 @@ vu_log_queue_fill(VuDev *dev, VuVirtq *vq, max = vq->vring.num; i = elem->index; - if (lduw_le_p(&desc[i].flags) & VRING_DESC_F_INDIRECT) { - if (ldl_le_p(&desc[i].len) % sizeof(struct vring_desc)) { + if (le16toh(desc[i].flags) & VRING_DESC_F_INDIRECT) { + if (le32toh(desc[i].len) % sizeof(struct vring_desc)) { vu_panic(dev, "Invalid size for indirect buffer table"); return; } /* loop over the indirect descriptor table */ - desc_addr = ldq_le_p(&desc[i].addr); - desc_len = ldl_le_p(&desc[i].len); + desc_addr = le64toh(desc[i].addr); + desc_len = le32toh(desc[i].len); max = desc_len / sizeof(struct vring_desc); read_len = desc_len; desc = vu_gpa_to_va(dev, &read_len, desc_addr); @@ -2765,9 +2768,9 @@ vu_log_queue_fill(VuDev *dev, VuVirtq *vq, return; } - if (lduw_le_p(&desc[i].flags) & VRING_DESC_F_WRITE) { - min = MIN(ldl_le_p(&desc[i].len), len); - vu_log_write(dev, ldq_le_p(&desc[i].addr), min); + if (le16toh(desc[i].flags) & VRING_DESC_F_WRITE) { + min = MIN(le32toh(desc[i].len), len); + vu_log_write(dev, le64toh(desc[i].addr), min); len -= min; } @@ -2792,15 +2795,15 @@ vu_queue_fill(VuDev *dev, VuVirtq *vq, idx = (idx + vq->used_idx) % vq->vring.num; - stl_le_p(&uelem.id, elem->index); - stl_le_p(&uelem.len, len); + uelem.id = htole32(elem->index); + uelem.len = htole32(len); vring_used_write(dev, vq, &uelem, idx); } static inline void vring_used_idx_set(VuDev *dev, VuVirtq *vq, uint16_t val) { - stw_le_p(&vq->vring.used->idx, val); + vq->vring.used->idx = htole16(val); vu_log_write(dev, vq->vring.log_guest_addr + offsetof(struct vring_used, idx), sizeof(vq->vring.used->idx)); From 810033be083040591fee2fc09f2f294670ed1930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:34 +0400 Subject: [PATCH 45/65] libvhost-user: replace qemu/memfd.h usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Undo the damage from commit 5f9ff1eff3 ("libvhost-user: Support tracking inflight I/O in shared memory") which introduced glib dependency through osdep.h inclusion. libvhost-user.c tries to stay free from glib usage. Use glibc memfd_create directly when available (assumed so when MFD_ALLOW_SEALING is defined). A following commit will make the project standalone and check for memfd API at configure time, instead of a panic at runtime. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Hajnoczi Message-Id: <20201125100640.366523-3-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/libvhost-user/libvhost-user.c | 50 +++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index 1c1cfbf1e7..54aabd1878 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -42,8 +42,6 @@ #endif #include "qemu/atomic.h" -#include "qemu/osdep.h" -#include "qemu/memfd.h" #include "libvhost-user.h" @@ -1615,11 +1613,45 @@ vu_inflight_queue_size(uint16_t queue_size) sizeof(uint16_t), INFLIGHT_ALIGNMENT); } +#ifdef MFD_ALLOW_SEALING +static void * +memfd_alloc(const char *name, size_t size, unsigned int flags, int *fd) +{ + void *ptr; + int ret; + + *fd = memfd_create(name, MFD_ALLOW_SEALING); + if (*fd < 0) { + return NULL; + } + + ret = ftruncate(*fd, size); + if (ret < 0) { + close(*fd); + return NULL; + } + + ret = fcntl(*fd, F_ADD_SEALS, flags); + if (ret < 0) { + close(*fd); + return NULL; + } + + ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, *fd, 0); + if (ptr == MAP_FAILED) { + close(*fd); + return NULL; + } + + return ptr; +} +#endif + static bool vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg) { - int fd; - void *addr; + int fd = -1; + void *addr = NULL; uint64_t mmap_size; uint16_t num_queues, queue_size; @@ -1637,9 +1669,13 @@ vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg) mmap_size = vu_inflight_queue_size(queue_size) * num_queues; - addr = qemu_memfd_alloc("vhost-inflight", mmap_size, - F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL, - &fd, NULL); +#ifdef MFD_ALLOW_SEALING + addr = memfd_alloc("vhost-inflight", mmap_size, + F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL, + &fd); +#else + vu_panic(dev, "Not implemented: memfd support is missing"); +#endif if (!addr) { vu_panic(dev, "Failed to alloc vhost inflight area"); From 7fa1d61695d92661c4800a7ee727ab7ae15a170b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:35 +0400 Subject: [PATCH 46/65] libvhost-user: remove qemu/compiler.h usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20201125100640.366523-4-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/libvhost-user/libvhost-user.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index 54aabd1878..fab7ca17ee 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -27,7 +27,6 @@ #include #include #include -#include "qemu/compiler.h" #if defined(__linux__) #include @@ -60,6 +59,10 @@ /* Round number up to multiple */ #define ALIGN_UP(n, m) ALIGN_DOWN((n) + (m) - 1, (m)) +#ifndef unlikely +#define unlikely(x) __builtin_expect(!!(x), 0) +#endif + /* Align each region to cache line size in inflight buffer */ #define INFLIGHT_ALIGNMENT 64 From 3d22bd27acd93281354b10c6bf787b720685eb80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:36 +0400 Subject: [PATCH 47/65] libvhost-user: drop qemu/osdep.h dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20201125100640.366523-5-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/libvhost-user/libvhost-user-glib.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/contrib/libvhost-user/libvhost-user-glib.c b/contrib/libvhost-user/libvhost-user-glib.c index 0df2ec9271..efc9d814e3 100644 --- a/contrib/libvhost-user/libvhost-user-glib.c +++ b/contrib/libvhost-user/libvhost-user-glib.c @@ -12,10 +12,16 @@ * later. See the COPYING file in the top-level directory. */ -#include "qemu/osdep.h" - #include "libvhost-user-glib.h" +#ifndef container_of +#define container_of(ptr, type, member) \ + __extension__({ \ + void *__mptr = (void *)(ptr); \ + ((type *)(__mptr - offsetof(type, member))); \ + }) +#endif + /* glib event loop integration for libvhost-user and misc callbacks */ G_STATIC_ASSERT((int)G_IO_IN == (int)VU_WATCH_IN); From 0df750e9d3a5fea5e19f4750582121c9293a9d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:37 +0400 Subject: [PATCH 48/65] libvhost-user: make it a meson subproject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By making libvhost-user a subproject, check it builds standalone (without the global QEMU cflags etc). Note that the library still relies on QEMU include/qemu/atomic.h and linux_headers/. Signed-off-by: Marc-André Lureau Message-Id: <20201125100640.366523-6-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- block/export/vhost-user-blk-server.c | 2 +- contrib/libvhost-user/meson.build | 4 ---- contrib/vhost-user-blk/meson.build | 3 +-- contrib/vhost-user-blk/vhost-user-blk.c | 3 +-- contrib/vhost-user-gpu/meson.build | 3 +-- contrib/vhost-user-gpu/vugpu.h | 2 +- contrib/vhost-user-input/main.c | 3 +-- contrib/vhost-user-input/meson.build | 3 +-- contrib/vhost-user-scsi/meson.build | 3 +-- contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +- include/qemu/vhost-user-server.h | 2 +- meson.build | 7 ++++++- .../libvhost-user/libvhost-user-glib.c | 0 .../libvhost-user/libvhost-user-glib.h | 0 .../libvhost-user/libvhost-user.c | 0 .../libvhost-user/libvhost-user.h | 0 subprojects/libvhost-user/meson.build | 20 +++++++++++++++++++ tests/meson.build | 3 +-- tests/vhost-user-bridge.c | 2 +- tools/virtiofsd/fuse_virtio.c | 2 +- tools/virtiofsd/meson.build | 3 +-- 21 files changed, 40 insertions(+), 27 deletions(-) delete mode 100644 contrib/libvhost-user/meson.build rename {contrib => subprojects}/libvhost-user/libvhost-user-glib.c (100%) rename {contrib => subprojects}/libvhost-user/libvhost-user-glib.h (100%) rename {contrib => subprojects}/libvhost-user/libvhost-user.c (100%) rename {contrib => subprojects}/libvhost-user/libvhost-user.h (100%) create mode 100644 subprojects/libvhost-user/meson.build diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c index 62672d1cb9..a3d95ca012 100644 --- a/block/export/vhost-user-blk-server.c +++ b/block/export/vhost-user-blk-server.c @@ -11,7 +11,7 @@ */ #include "qemu/osdep.h" #include "block/block.h" -#include "contrib/libvhost-user/libvhost-user.h" +#include "subprojects/libvhost-user/libvhost-user.h" /* only for the type definitions */ #include "standard-headers/linux/virtio_blk.h" #include "qemu/vhost-user-server.h" #include "vhost-user-blk-server.h" diff --git a/contrib/libvhost-user/meson.build b/contrib/libvhost-user/meson.build deleted file mode 100644 index a261e7665f..0000000000 --- a/contrib/libvhost-user/meson.build +++ /dev/null @@ -1,4 +0,0 @@ -libvhost_user = static_library('vhost-user', - files('libvhost-user.c', 'libvhost-user-glib.c'), - build_by_default: false) -vhost_user = declare_dependency(link_with: libvhost_user) diff --git a/contrib/vhost-user-blk/meson.build b/contrib/vhost-user-blk/meson.build index 5db8cc3fe2..601ea15ef5 100644 --- a/contrib/vhost-user-blk/meson.build +++ b/contrib/vhost-user-blk/meson.build @@ -1,6 +1,5 @@ # FIXME: broken on 32-bit architectures executable('vhost-user-blk', files('vhost-user-blk.c'), - link_with: libvhost_user, - dependencies: qemuutil, + dependencies: [qemuutil, vhost_user], build_by_default: false, install: false) diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c index dc981bf945..6abd7835a8 100644 --- a/contrib/vhost-user-blk/vhost-user-blk.c +++ b/contrib/vhost-user-blk/vhost-user-blk.c @@ -17,8 +17,7 @@ #include "qemu/osdep.h" #include "standard-headers/linux/virtio_blk.h" -#include "contrib/libvhost-user/libvhost-user-glib.h" -#include "contrib/libvhost-user/libvhost-user.h" +#include "libvhost-user-glib.h" #if defined(__linux__) #include diff --git a/contrib/vhost-user-gpu/meson.build b/contrib/vhost-user-gpu/meson.build index c487ca72c1..2fc2320b52 100644 --- a/contrib/vhost-user-gpu/meson.build +++ b/contrib/vhost-user-gpu/meson.build @@ -2,8 +2,7 @@ if 'CONFIG_TOOLS' in config_host and 'CONFIG_VIRGL' in config_host \ and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host \ and pixman.found() executable('vhost-user-gpu', files('vhost-user-gpu.c', 'virgl.c', 'vugbm.c'), - link_with: libvhost_user, - dependencies: [qemuutil, pixman, gbm, virgl], + dependencies: [qemuutil, pixman, gbm, virgl, vhost_user], install: true, install_dir: get_option('libexecdir')) diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h index 3153c9a6de..bdf9a74b46 100644 --- a/contrib/vhost-user-gpu/vugpu.h +++ b/contrib/vhost-user-gpu/vugpu.h @@ -17,7 +17,7 @@ #include "qemu/osdep.h" -#include "contrib/libvhost-user/libvhost-user-glib.h" +#include "libvhost-user-glib.h" #include "standard-headers/linux/virtio_gpu.h" #include "qemu/queue.h" diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c index 6020c6f33a..3ea840cf44 100644 --- a/contrib/vhost-user-input/main.c +++ b/contrib/vhost-user-input/main.c @@ -12,8 +12,7 @@ #include "qemu/iov.h" #include "qemu/bswap.h" #include "qemu/sockets.h" -#include "contrib/libvhost-user/libvhost-user.h" -#include "contrib/libvhost-user/libvhost-user-glib.h" +#include "libvhost-user-glib.h" #include "standard-headers/linux/virtio_input.h" #include "qapi/error.h" diff --git a/contrib/vhost-user-input/meson.build b/contrib/vhost-user-input/meson.build index 1eeb1329d9..21a9ed4f15 100644 --- a/contrib/vhost-user-input/meson.build +++ b/contrib/vhost-user-input/meson.build @@ -1,5 +1,4 @@ executable('vhost-user-input', files('main.c'), - link_with: libvhost_user, - dependencies: qemuutil, + dependencies: [qemuutil, vhost_user], build_by_default: targetos == 'linux', install: false) diff --git a/contrib/vhost-user-scsi/meson.build b/contrib/vhost-user-scsi/meson.build index 257cbffc8e..044c50bf43 100644 --- a/contrib/vhost-user-scsi/meson.build +++ b/contrib/vhost-user-scsi/meson.build @@ -1,7 +1,6 @@ if 'CONFIG_LIBISCSI' in config_host executable('vhost-user-scsi', files('vhost-user-scsi.c'), - link_with: libvhost_user, - dependencies: [qemuutil, libiscsi], + dependencies: [qemuutil, libiscsi, vhost_user], build_by_default: targetos == 'linux', install: false) endif diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c index 4639440a70..4f6e3e2a24 100644 --- a/contrib/vhost-user-scsi/vhost-user-scsi.c +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c @@ -15,7 +15,7 @@ #define inline __attribute__((gnu_inline)) /* required for libiscsi v1.9.0 */ #include #undef inline -#include "contrib/libvhost-user/libvhost-user-glib.h" +#include "libvhost-user-glib.h" #include "standard-headers/linux/virtio_scsi.h" diff --git a/include/qemu/vhost-user-server.h b/include/qemu/vhost-user-server.h index 0da4c2cc4c..121ea1dedf 100644 --- a/include/qemu/vhost-user-server.h +++ b/include/qemu/vhost-user-server.h @@ -11,7 +11,7 @@ #ifndef VHOST_USER_SERVER_H #define VHOST_USER_SERVER_H -#include "contrib/libvhost-user/libvhost-user.h" +#include "subprojects/libvhost-user/libvhost-user.h" /* only for the type definitions */ #include "io/channel-socket.h" #include "io/channel-file.h" #include "io/net-listener.h" diff --git a/meson.build b/meson.build index e3386196ba..732b29a1f3 100644 --- a/meson.build +++ b/meson.build @@ -1475,7 +1475,12 @@ trace_events_subdirs += [ 'util', ] -subdir('contrib/libvhost-user') +vhost_user = not_found +if 'CONFIG_VHOST_USER' in config_host + libvhost_user = subproject('libvhost-user') + vhost_user = libvhost_user.get_variable('vhost_user_dep') +endif + subdir('qapi') subdir('qobject') subdir('stubs') diff --git a/contrib/libvhost-user/libvhost-user-glib.c b/subprojects/libvhost-user/libvhost-user-glib.c similarity index 100% rename from contrib/libvhost-user/libvhost-user-glib.c rename to subprojects/libvhost-user/libvhost-user-glib.c diff --git a/contrib/libvhost-user/libvhost-user-glib.h b/subprojects/libvhost-user/libvhost-user-glib.h similarity index 100% rename from contrib/libvhost-user/libvhost-user-glib.h rename to subprojects/libvhost-user/libvhost-user-glib.h diff --git a/contrib/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c similarity index 100% rename from contrib/libvhost-user/libvhost-user.c rename to subprojects/libvhost-user/libvhost-user.c diff --git a/contrib/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h similarity index 100% rename from contrib/libvhost-user/libvhost-user.h rename to subprojects/libvhost-user/libvhost-user.h diff --git a/subprojects/libvhost-user/meson.build b/subprojects/libvhost-user/meson.build new file mode 100644 index 0000000000..f9ecc534cf --- /dev/null +++ b/subprojects/libvhost-user/meson.build @@ -0,0 +1,20 @@ +project('libvhost-user', 'c', + license: 'GPL-2.0-or-later', + default_options: ['c_std=gnu99']) + +glib = dependency('glib-2.0') +inc = include_directories('../../include', '../../linux-headers') + +vhost_user = static_library('vhost-user', + files('libvhost-user.c'), + include_directories: inc, + c_args: '-D_GNU_SOURCE') + +vhost_user_glib = static_library('vhost-user-glib', + files('libvhost-user-glib.c'), + include_directories: inc, + link_with: vhost_user, + dependencies: glib) + +vhost_user_dep = declare_dependency(link_with: vhost_user_glib, + include_directories: include_directories('.')) diff --git a/tests/meson.build b/tests/meson.build index afeb6be689..1fa068f27b 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -264,8 +264,7 @@ endforeach if have_tools and 'CONFIG_VHOST_USER' in config_host and 'CONFIG_LINUX' in config_host executable('vhost-user-bridge', sources: files('vhost-user-bridge.c'), - link_with: [libvhost_user], - dependencies: [qemuutil]) + dependencies: [qemuutil, vhost_user]) endif if have_system and 'CONFIG_POSIX' in config_host diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c index bd43607a4d..24815920b2 100644 --- a/tests/vhost-user-bridge.c +++ b/tests/vhost-user-bridge.c @@ -34,7 +34,7 @@ #include "qemu/ctype.h" #include "qemu/iov.h" #include "standard-headers/linux/virtio_net.h" -#include "contrib/libvhost-user/libvhost-user.h" +#include "libvhost-user.h" #define VHOST_USER_BRIDGE_DEBUG 1 diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index 83ba07c6cd..623812c432 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -35,7 +35,7 @@ #include #include -#include "contrib/libvhost-user/libvhost-user.h" +#include "libvhost-user.h" struct fv_VuDev; struct fv_QueueInfo { diff --git a/tools/virtiofsd/meson.build b/tools/virtiofsd/meson.build index 17edecf55c..c134ba633f 100644 --- a/tools/virtiofsd/meson.build +++ b/tools/virtiofsd/meson.build @@ -8,8 +8,7 @@ executable('virtiofsd', files( 'helper.c', 'passthrough_ll.c', 'passthrough_seccomp.c'), - link_with: libvhost_user, - dependencies: [seccomp, qemuutil, libcap_ng], + dependencies: [seccomp, qemuutil, libcap_ng, vhost_user], install: true, install_dir: get_option('libexecdir')) From e0193568daf1785772c9d97a9825c401d463865f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:39 +0400 Subject: [PATCH 49/65] libvhost-user: add a simple link test without glib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20201125100640.366523-8-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- subprojects/libvhost-user/link-test.c | 45 +++++++++++++++++++++++++++ subprojects/libvhost-user/meson.build | 4 +++ 2 files changed, 49 insertions(+) create mode 100644 subprojects/libvhost-user/link-test.c diff --git a/subprojects/libvhost-user/link-test.c b/subprojects/libvhost-user/link-test.c new file mode 100644 index 0000000000..e01d6eb1fa --- /dev/null +++ b/subprojects/libvhost-user/link-test.c @@ -0,0 +1,45 @@ +/* + * A trivial unit test to check linking without glib. A real test suite should + * probably based off libvhost-user-glib instead. + */ +#include +#include +#include "libvhost-user.h" + +static void +panic(VuDev *dev, const char *err) +{ + abort(); +} + +static void +set_watch(VuDev *dev, int fd, int condition, + vu_watch_cb cb, void *data) +{ + abort(); +} + +static void +remove_watch(VuDev *dev, int fd) +{ + abort(); +} + +static const VuDevIface iface = { + 0, +}; + +int +main(int argc, const char *argv[]) +{ + bool rc; + uint16_t max_queues = 2; + int socket = 0; + VuDev dev = { 0, }; + + rc = vu_init(&dev, max_queues, socket, panic, NULL, set_watch, remove_watch, &iface); + assert(rc == true); + vu_deinit(&dev); + + return 0; +} diff --git a/subprojects/libvhost-user/meson.build b/subprojects/libvhost-user/meson.build index f9ecc534cf..c5d85c11d7 100644 --- a/subprojects/libvhost-user/meson.build +++ b/subprojects/libvhost-user/meson.build @@ -10,6 +10,10 @@ vhost_user = static_library('vhost-user', include_directories: inc, c_args: '-D_GNU_SOURCE') +executable('link-test', files('link-test.c'), + link_whole: vhost_user, + include_directories: inc) + vhost_user_glib = static_library('vhost-user-glib', files('libvhost-user-glib.c'), include_directories: inc, From c4698e360cf9e29fdbd37d0bd0c248fe2e6d3c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2020 14:06:40 +0400 Subject: [PATCH 50/65] .gitlab-ci: add build-libvhost-user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20201125100640.366523-9-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- .gitlab-ci.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d0173e82b1..e517506c35 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -455,6 +455,17 @@ check-dco: variables: GIT_DEPTH: 1000 +build-libvhost-user: + stage: build + image: $CI_REGISTRY_IMAGE/qemu/fedora:latest + before_script: + - dnf install -y meson ninja-build + script: + - mkdir subprojects/libvhost-user/build + - cd subprojects/libvhost-user/build + - meson + - ninja + pages: image: $CI_REGISTRY_IMAGE/qemu/ubuntu2004:latest stage: test From acb1f3c248a83fb66d705068ed7d098b898632b1 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 18 Nov 2020 09:16:41 +0000 Subject: [PATCH 51/65] contrib/vhost-user-blk: avoid g_return_val_if() input validation Do not validate input with g_return_val_if(). This API is intended for checking programming errors and is compiled out with -DG_DISABLE_CHECKS. Use an explicit if statement for input validation so it cannot accidentally be compiled out. Suggested-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi Message-Id: <20201118091644.199527-2-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-blk/vhost-user-blk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c index 6abd7835a8..d14b2896bf 100644 --- a/contrib/vhost-user-blk/vhost-user-blk.c +++ b/contrib/vhost-user-blk/vhost-user-blk.c @@ -403,7 +403,9 @@ vub_get_config(VuDev *vu_dev, uint8_t *config, uint32_t len) VugDev *gdev; VubDev *vdev_blk; - g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1); + if (len > sizeof(struct virtio_blk_config)) { + return -1; + } gdev = container_of(vu_dev, VugDev, parent); vdev_blk = container_of(gdev, VubDev, parent); From fa77464ffe14be9532b861b33f57f5c1d63a4824 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 18 Nov 2020 09:16:42 +0000 Subject: [PATCH 52/65] contrib/vhost-user-gpu: avoid g_return_val_if() input validation Do not validate input with g_return_val_if(). This API is intended for checking programming errors and is compiled out with -DG_DISABLE_CHECKS. Use an explicit if statement for input validation so it cannot accidentally be compiled out. Suggested-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi Message-Id: <20201118091644.199527-3-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-gpu/vhost-user-gpu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c index a019d0a9ac..f445ef28ec 100644 --- a/contrib/vhost-user-gpu/vhost-user-gpu.c +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c @@ -1044,7 +1044,9 @@ vg_get_config(VuDev *dev, uint8_t *config, uint32_t len) { VuGpu *g = container_of(dev, VuGpu, dev.parent); - g_return_val_if_fail(len <= sizeof(struct virtio_gpu_config), -1); + if (len > sizeof(struct virtio_gpu_config)) { + return -1; + } if (opt_virgl) { g->virtio_config.num_capsets = vg_virgl_get_num_capsets(); From a606169aca0f086d3835996ca74e2b7b87cd9df5 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 18 Nov 2020 09:16:43 +0000 Subject: [PATCH 53/65] contrib/vhost-user-input: avoid g_return_val_if() input validation Do not validate input with g_return_val_if(). This API is intended for checking programming errors and is compiled out with -DG_DISABLE_CHECKS. Use an explicit if statement for input validation so it cannot accidentally be compiled out. Suggested-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi Message-Id: <20201118091644.199527-4-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-input/main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c index 3ea840cf44..d2de47cee7 100644 --- a/contrib/vhost-user-input/main.c +++ b/contrib/vhost-user-input/main.c @@ -211,7 +211,9 @@ static int vi_get_config(VuDev *dev, uint8_t *config, uint32_t len) { VuInput *vi = container_of(dev, VuInput, dev.parent); - g_return_val_if_fail(len <= sizeof(*vi->sel_config), -1); + if (len > sizeof(*vi->sel_config)) { + return -1; + } if (vi->sel_config) { memcpy(config, vi->sel_config, len); From 552c2c4c101335e67ac8ae8ab022d88a8795f2ac Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 18 Nov 2020 09:16:44 +0000 Subject: [PATCH 54/65] block/export: avoid g_return_val_if() input validation Do not validate input with g_return_val_if(). This API is intended for checking programming errors and is compiled out with -DG_DISABLE_CHECKS. Use an explicit if statement for input validation so it cannot accidentally be compiled out. Suggested-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi Message-Id: <20201118091644.199527-5-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- block/export/vhost-user-blk-server.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c index a3d95ca012..ab2c4d44c4 100644 --- a/block/export/vhost-user-blk-server.c +++ b/block/export/vhost-user-blk-server.c @@ -267,7 +267,9 @@ vu_blk_get_config(VuDev *vu_dev, uint8_t *config, uint32_t len) VuServer *server = container_of(vu_dev, VuServer, vu_dev); VuBlkExport *vexp = container_of(server, VuBlkExport, vu_server); - g_return_val_if_fail(len <= sizeof(struct virtio_blk_config), -1); + if (len > sizeof(struct virtio_blk_config)) { + return -1; + } memcpy(config, &vexp->blkcfg, len); return 0; From 0657c657eb37bb48bfd9fe3ae8a323ae3455f47b Mon Sep 17 00:00:00 2001 From: Erich-McMillan Date: Tue, 8 Dec 2020 09:53:38 -0600 Subject: [PATCH 55/65] hw/i386/pc: add max combined fw size as machine configuration option At Hewlett Packard Inc. we have a need for increased fw size to enable testing of our custom fw. Rebase v6 patch to d73c46e4 Signed-off-by: Erich McMillan Message-Id: <20201208155338.14-1-erich.mcmillan@hp.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ hw/i386/pc_sysfw.c | 15 +++---------- include/hw/i386/pc.h | 2 ++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 7113fb0770..675e15c0aa 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1569,6 +1569,50 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v, pcms->max_ram_below_4g = value; } +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + PCMachineState *pcms = PC_MACHINE(obj); + uint64_t value = pcms->max_fw_size; + + visit_type_size(v, name, &value, errp); +} + +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + PCMachineState *pcms = PC_MACHINE(obj); + Error *error = NULL; + uint64_t value; + + visit_type_size(v, name, &value, &error); + if (error) { + error_propagate(errp, error); + return; + } + + /* + * We don't have a theoretically justifiable exact lower bound on the base + * address of any flash mapping. In practice, the IO-APIC MMIO range is + * [0xFEE00000..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free + * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in + * size. + */ + if (value > 16 * MiB) { + error_setg(errp, + "User specified max allowed firmware size %" PRIu64 " is " + "greater than 16MiB. If combined firwmare size exceeds " + "16MiB the system may not boot, or experience intermittent" + "stability issues.", + value); + return; + } + + pcms->max_fw_size = value; +} + static void pc_machine_initfn(Object *obj) { PCMachineState *pcms = PC_MACHINE(obj); @@ -1584,6 +1628,7 @@ static void pc_machine_initfn(Object *obj) pcms->smbus_enabled = true; pcms->sata_enabled = true; pcms->pit_enabled = true; + pcms->max_fw_size = 8 * MiB; #ifdef CONFIG_HPET pcms->hpet_enabled = true; #endif @@ -1710,6 +1755,12 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) object_class_property_add_bool(oc, "hpet", pc_machine_get_hpet, pc_machine_set_hpet); + + object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size", + pc_machine_get_max_fw_size, pc_machine_set_max_fw_size, + NULL, NULL); + object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE, + "Maximum combined firmware size"); } static const TypeInfo pc_machine_info = { diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index b6c0822fe3..f8bd3a8b85 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -39,15 +39,6 @@ #include "hw/block/flash.h" #include "sysemu/kvm.h" -/* - * We don't have a theoretically justifiable exact lower bound on the base - * address of any flash mapping. In practice, the IO-APIC MMIO range is - * [0xFEE00000..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in - * size. - */ -#define FLASH_SIZE_LIMIT (8 * MiB) - #define FLASH_SECTOR_SIZE 4096 static void pc_isa_bios_init(MemoryRegion *rom_memory, @@ -140,7 +131,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms) * Stop at the first pcms->flash[0] lacking a block backend. * Set each flash's size from its block backend. Fatal error if the * size isn't a non-zero multiple of 4KiB, or the total size exceeds - * FLASH_SIZE_LIMIT. + * pcms->max_fw_size. * * If pcms->flash[0] has a block backend, its memory is passed to * pc_isa_bios_init(). Merging several flash devices for isa-bios is @@ -182,10 +173,10 @@ static void pc_system_flash_map(PCMachineState *pcms, } if ((hwaddr)size != size || total_size > HWADDR_MAX - size - || total_size + size > FLASH_SIZE_LIMIT) { + || total_size + size > pcms->max_fw_size) { error_report("combined size of system firmware exceeds " "%" PRIu64 " bytes", - FLASH_SIZE_LIMIT); + pcms->max_fw_size); exit(1); } diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 49dfa667de..2aa8797c6e 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -44,6 +44,7 @@ typedef struct PCMachineState { bool sata_enabled; bool pit_enabled; bool hpet_enabled; + uint64_t max_fw_size; /* NUMA information: */ uint64_t numa_nodes; @@ -60,6 +61,7 @@ typedef struct PCMachineState { #define PC_MACHINE_SMBUS "smbus" #define PC_MACHINE_SATA "sata" #define PC_MACHINE_PIT "pit" +#define PC_MACHINE_MAX_FW_SIZE "max-fw-size" /** * PCMachineClass: From 1e6107d901ca3cb31ee3dbf3d55b1778682f0979 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 7 Dec 2020 09:07:33 -0500 Subject: [PATCH 56/65] acpi: cpuhp: introduce 'firmware performs eject' status/control bits Adds bit #4 to status/control field of CPU hotplug MMIO interface. New bit will be used OSPM to mark CPUs as pending for removal by firmware, when it calls _EJ0 method on CPU device node. Later on, when firmware sees this bit set, it will perform CPU eject which will clear bit #4 as well. Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- docs/specs/acpi_cpu_hotplug.txt | 19 ++++++++++++++----- hw/acpi/cpu.c | 12 +++++++++++- hw/acpi/trace-events | 2 ++ include/hw/acpi/cpu.h | 1 + 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt index 9bb22d1270..9bd59ae0da 100644 --- a/docs/specs/acpi_cpu_hotplug.txt +++ b/docs/specs/acpi_cpu_hotplug.txt @@ -56,8 +56,11 @@ read access: no device check event to OSPM was issued. It's valid only when bit 0 is set. 2: Device remove event, used to distinguish device for which - no device eject request to OSPM was issued. - 3-7: reserved and should be ignored by OSPM + no device eject request to OSPM was issued. Firmware must + ignore this bit. + 3: reserved and should be ignored by OSPM + 4: if set to 1, OSPM requests firmware to perform device eject. + 5-7: reserved and should be ignored by OSPM [0x5-0x7] reserved [0x8] Command data: (DWORD access) contains 0 unless value last stored in 'Command field' is one of: @@ -79,10 +82,16 @@ write access: selected CPU device 2: if set to 1 clears device remove event, set by OSPM after it has emitted device eject request for the - selected CPU device + selected CPU device. 3: if set to 1 initiates device eject, set by OSPM when it - triggers CPU device removal and calls _EJ0 method - 4-7: reserved, OSPM must clear them before writing to register + triggers CPU device removal and calls _EJ0 method or by firmware + when bit #4 is set. In case bit #4 were set, it's cleared as + part of device eject. + 4: if set to 1, OSPM hands over device eject to firmware. + Firmware shall issue device eject request as described above + (bit #3) and OSPM should not touch device eject bit (#3) in case + it's asked firmware to perform CPU device eject. + 5-7: reserved, OSPM must clear them before writing to register [0x5] Command field: (1 byte access) value: 0: selects a CPU device with inserting/removing events and diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index f099b50927..1293204438 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -71,6 +71,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, unsigned size) val |= cdev->cpu ? 1 : 0; val |= cdev->is_inserting ? 2 : 0; val |= cdev->is_removing ? 4 : 0; + val |= cdev->fw_remove ? 16 : 0; trace_cpuhp_acpi_read_flags(cpu_st->selector, val); break; case ACPI_CPU_CMD_DATA_OFFSET_RW: @@ -148,6 +149,14 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data, hotplug_ctrl = qdev_get_hotplug_handler(dev); hotplug_handler_unplug(hotplug_ctrl, dev, NULL); object_unparent(OBJECT(dev)); + cdev->fw_remove = false; + } else if (data & 16) { + if (!cdev->cpu || cdev->cpu == first_cpu) { + trace_cpuhp_acpi_fw_remove_invalid_cpu(cpu_st->selector); + break; + } + trace_cpuhp_acpi_fw_remove_cpu(cpu_st->selector); + cdev->fw_remove = true; } break; case ACPI_CPU_CMD_OFFSET_WR: @@ -159,7 +168,8 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, uint64_t data, do { cdev = &cpu_st->devs[iter]; - if (cdev->is_inserting || cdev->is_removing) { + if (cdev->is_inserting || cdev->is_removing || + cdev->fw_remove) { cpu_st->selector = iter; trace_cpuhp_acpi_cpu_has_events(cpu_st->selector, cdev->is_inserting, cdev->is_removing); diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events index afbc77de1c..f91ced477d 100644 --- a/hw/acpi/trace-events +++ b/hw/acpi/trace-events @@ -29,6 +29,8 @@ cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]" cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]" cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32 cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32 +cpuhp_acpi_fw_remove_invalid_cpu(uint32_t idx) "0x%"PRIx32 +cpuhp_acpi_fw_remove_cpu(uint32_t idx) "0x%"PRIx32 cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32 cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32 diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h index 0eeedaa491..d71edde456 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -22,6 +22,7 @@ typedef struct AcpiCpuStatus { uint64_t arch_id; bool is_inserting; bool is_removing; + bool fw_remove; uint32_t ost_event; uint32_t ost_status; } AcpiCpuStatus; From 892aae74306c9a5e3859c1eef6ed430e1a759bfd Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 7 Dec 2020 09:07:34 -0500 Subject: [PATCH 57/65] x86: acpi: introduce AcpiPmInfo::smi_on_cpu_unplug Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-4-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 76e27f8fad..44c9da5112 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -96,6 +96,7 @@ typedef struct AcpiPmInfo { bool s4_disabled; bool pcihp_bridge_en; bool smi_on_cpuhp; + bool smi_on_cpu_unplug; bool pcihp_root_en; uint8_t s4_val; AcpiFadtData fadt; @@ -197,6 +198,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm) pm->pcihp_io_base = 0; pm->pcihp_io_len = 0; pm->smi_on_cpuhp = false; + pm->smi_on_cpu_unplug = false; assert(obj); init_common_fadt_data(machine, obj, &pm->fadt); @@ -220,6 +222,8 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm) pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE; pm->smi_on_cpuhp = !!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT)); + pm->smi_on_cpu_unplug = + !!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)); } /* The above need not be conditional on machine type because the reset port From 414aa64fda6099eefebb2fb7bd4a0350b34738cc Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 7 Dec 2020 09:07:35 -0500 Subject: [PATCH 58/65] tests/acpi: allow expected files change Change that will be introduced by following patch: @@ -557,6 +557,7 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001) CINS, 1, CRMV, 1, CEJ0, 1, + CEJF, 1, Offset (0x05), CCMD, 8 } Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-5-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..cc75f3fc46 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,22 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/pc/DSDT", +"tests/data/acpi/q35/DSDT", +"tests/data/acpi/q35/DSDT.tis", +"tests/data/acpi/q35/DSDT.bridge", +"tests/data/acpi/q35/DSDT.mmio64", +"tests/data/acpi/q35/DSDT.ipmibt", +"tests/data/acpi/q35/DSDT.cphp", +"tests/data/acpi/q35/DSDT.memhp", +"tests/data/acpi/q35/DSDT.numamem", +"tests/data/acpi/q35/DSDT.dimmpxm", +"tests/data/acpi/q35/DSDT.acpihmat", +"tests/data/acpi/pc/DSDT.bridge", +"tests/data/acpi/pc/DSDT.ipmikcs", +"tests/data/acpi/pc/DSDT.cphp", +"tests/data/acpi/pc/DSDT.memhp", +"tests/data/acpi/pc/DSDT.numamem", +"tests/data/acpi/pc/DSDT.dimmpxm", +"tests/data/acpi/pc/DSDT.acpihmat", +"tests/data/acpi/pc/DSDT.roothp", +"tests/data/acpi/pc/DSDT.hpbridge", +"tests/data/acpi/pc/DSDT.hpbrroot", From 69dea9d6b3d1070ee49498a18c9bee5b60d00619 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 7 Dec 2020 09:07:36 -0500 Subject: [PATCH 59/65] x86: acpi: let the firmware handle pending "CPU remove" events in SMM if firmware and QEMU negotiated CPU hotunplug support, generate _EJ0 method so that it will mark CPU for removal by firmware and pass control to it by triggering SMI. Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-6-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/cpu.c | 14 ++++++++++++-- hw/i386/acpi-build.c | 1 + include/hw/acpi/cpu.h | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index 1293204438..6350caa765 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -342,6 +342,7 @@ const VMStateDescription vmstate_cpu_hotplug = { #define CPU_INSERT_EVENT "CINS" #define CPU_REMOVE_EVENT "CRMV" #define CPU_EJECT_EVENT "CEJ0" +#define CPU_FW_EJECT_EVENT "CEJF" void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, hwaddr io_base, @@ -394,7 +395,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1)); /* initiates device eject, write only */ aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1)); - aml_append(field, aml_reserved_field(4)); + /* tell firmware to do device eject, write only */ + aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1)); + aml_append(field, aml_reserved_field(3)); aml_append(field, aml_named_field(CPU_COMMAND, 8)); aml_append(cpu_ctrl_dev, field); @@ -429,6 +432,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT); Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT); Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT); + Aml *fw_ej_evt = aml_name("%s.%s", cphp_res_path, CPU_FW_EJECT_EVENT); aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010"))); aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05"))); @@ -471,7 +475,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(method, aml_acquire(ctrl_lock, 0xFFFF)); aml_append(method, aml_store(idx, cpu_selector)); - aml_append(method, aml_store(one, ej_evt)); + if (opts.fw_unplugs_cpu) { + aml_append(method, aml_store(one, fw_ej_evt)); + aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD), + aml_name("%s", opts.smi_path))); + } else { + aml_append(method, aml_store(one, ej_evt)); + } aml_append(method, aml_release(ctrl_lock)); } aml_append(cpus_dev, method); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 44c9da5112..f18b71dea9 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1293,6 +1293,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, CPUHotplugFeatures opts = { .acpi_1_compatible = true, .has_legacy_cphp = true, .smi_path = pm->smi_on_cpuhp ? "\\_SB.PCI0.SMI0.SMIC" : NULL, + .fw_unplugs_cpu = pm->smi_on_cpu_unplug, }; build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_GPE._E02"); diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h index d71edde456..999caaf510 100644 --- a/include/hw/acpi/cpu.h +++ b/include/hw/acpi/cpu.h @@ -51,6 +51,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner, typedef struct CPUHotplugFeatures { bool acpi_1_compatible; bool has_legacy_cphp; + bool fw_unplugs_cpu; const char *smi_path; } CPUHotplugFeatures; From e2487e4028d76276eb63a450e27a059793a5ebf4 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 7 Dec 2020 09:07:37 -0500 Subject: [PATCH 60/65] tests/acpi: update expected files update expected files with following change: @@ -557,6 +557,7 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001) CINS, 1, CRMV, 1, CEJ0, 1, + CEJF, 1, Offset (0x05), CCMD, 8 } Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-7-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/pc/DSDT | Bin 5060 -> 5065 bytes tests/data/acpi/pc/DSDT.acpihmat | Bin 6385 -> 6390 bytes tests/data/acpi/pc/DSDT.bridge | Bin 6919 -> 6924 bytes tests/data/acpi/pc/DSDT.cphp | Bin 5524 -> 5529 bytes tests/data/acpi/pc/DSDT.dimmpxm | Bin 6714 -> 6719 bytes tests/data/acpi/pc/DSDT.hpbridge | Bin 5021 -> 5026 bytes tests/data/acpi/pc/DSDT.hpbrroot | Bin 3079 -> 3084 bytes tests/data/acpi/pc/DSDT.ipmikcs | Bin 5132 -> 5137 bytes tests/data/acpi/pc/DSDT.memhp | Bin 6419 -> 6424 bytes tests/data/acpi/pc/DSDT.numamem | Bin 5066 -> 5071 bytes tests/data/acpi/pc/DSDT.roothp | Bin 5256 -> 5261 bytes tests/data/acpi/q35/DSDT | Bin 7796 -> 7801 bytes tests/data/acpi/q35/DSDT.acpihmat | Bin 9121 -> 9126 bytes tests/data/acpi/q35/DSDT.bridge | Bin 7814 -> 7819 bytes tests/data/acpi/q35/DSDT.cphp | Bin 8260 -> 8265 bytes tests/data/acpi/q35/DSDT.dimmpxm | Bin 9450 -> 9455 bytes tests/data/acpi/q35/DSDT.ipmibt | Bin 7871 -> 7876 bytes tests/data/acpi/q35/DSDT.memhp | Bin 9155 -> 9160 bytes tests/data/acpi/q35/DSDT.mmio64 | Bin 8927 -> 8932 bytes tests/data/acpi/q35/DSDT.numamem | Bin 7802 -> 7807 bytes tests/data/acpi/q35/DSDT.tis | Bin 8402 -> 8407 bytes tests/qtest/bios-tables-test-allowed-diff.h | 21 -------------------- 22 files changed, 21 deletions(-) diff --git a/tests/data/acpi/pc/DSDT b/tests/data/acpi/pc/DSDT index 4ca46e5a2bdb1dfab79dd8630aeeb9a386d8b30e..f6173df1d598767a79aa34ad7585ad7d45c5d4f3 100644 GIT binary patch delta 74 zcmX@2eo~#wCDmuQ`QIw3J!5(P;d@#^<#AQ b^b2Nm4)P6SbawSJ01oMbU33dtLk!E0Ee6f*h3md1uU3{=pd~}n?0|1#g5zGJp diff --git a/tests/data/acpi/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm index e015b4594c96a6e0f34c0668e3383b9a91dff38e..c44385cc01879324738ffb7f997b8cdd762cbf97 100644 GIT binary patch delta 74 zcmdmGvfqTuCD@~J diff --git a/tests/data/acpi/pc/DSDT.hpbridge b/tests/data/acpi/pc/DSDT.hpbridge index 56032bcf1ba4e251f16c9028429826090531efdd..4ecf1eb13bf49499f729b53a6d0114672a76e28d 100644 GIT binary patch delta 74 zcmbQMzDS+RCDuQ`QIw3J!5(P;d@#^<#AQ b^b2Nm4)P6SbawSJ01A;%M( delta 69 zcmZ3aK3AQ~CD=y(9?$i@R delta 69 zcmbPXG}(yDCD@Zg!6A+e3eEwpevHnZ Ye!+~+LB3&(&aPetj0`NBJ=k4&0kY;2I{*Lx diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT index e7414e78563372fca4d2aab9d16c58c0ff8468f4..d25cd7072932886d6967f4023faac1e1fa6e836c 100644 GIT binary patch delta 74 zcmexj^V5dQCD delta 69 zcmexq^TmeCCD-hQE4OB1PM+ryZB(I_~<6*$%`f8W62|}l diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge index 118476ff6101e11d6b1f2d3399241d7fd1a6f634..06bac139d668ddfc7914e258b471a303c9dbd192 100644 GIT binary patch delta 74 zcmZp(?Y8A|33dtTmSbRG?BB>WL4woYE@Zg!6A+e3eEwpevHnZ Ye!+~+LB3&(&aPetj0`NBRV33{0jsML6aWAK diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp index 69c5edf620529e995461ccba63b76a083f25b2b6..2b933ac482e6883efccbd7d6c96089602f2c0b4d 100644 GIT binary patch delta 74 zcmX@&aMFRxCD&WMJN$ IB^k;J0IQk~=Kufz delta 47 zcmX?NyWf_}CD;VA-568OjO( DX1NXL diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp index 85598ca3f68f437e8d5048e2cb9815f20b332152..9a802e4c67022386442976d5cb997ea3fc57b58f 100644 GIT binary patch delta 74 zcmX@?e!`v0CDh($ delta 69 zcmaFjdf%1HCD Date: Mon, 7 Dec 2020 09:07:38 -0500 Subject: [PATCH 61/65] x86: ich9: factor out "guest_cpu_hotplug_features" it will be reused by next patch to check validity of unplug feature. Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-8-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/isa/lpc_ich9.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 087a18d04d..da80430144 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -366,6 +366,7 @@ static void smi_features_ok_callback(void *opaque) { ICH9LPCState *lpc = opaque; uint64_t guest_features; + uint64_t guest_cpu_hotplug_features; if (lpc->smi_features_ok) { /* negotiation already complete, features locked */ @@ -378,9 +379,12 @@ static void smi_features_ok_callback(void *opaque) /* guest requests invalid features, leave @features_ok at zero */ return; } + + guest_cpu_hotplug_features = guest_features & + (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) | + BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)); if (!(guest_features & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT)) && - guest_features & (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) | - BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) { + guest_cpu_hotplug_features) { /* * cpu hot-[un]plug with SMI requires SMI broadcast, * leave @features_ok at zero From 7ed3e1ebcbe542c74c32e842689af60d644fcee6 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 7 Dec 2020 09:07:39 -0500 Subject: [PATCH 62/65] x86: ich9: let firmware negotiate 'CPU hot-unplug with SMI' feature Keep CPU hotunplug with SMI disabled on 5.2 and older and enable it by default on newer machine types. Signed-off-by: Igor Mammedov Message-Id: <20201207140739.3829993-9-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 4 +++- hw/isa/lpc_ich9.c | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 675e15c0aa..9e29f3792b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -97,7 +97,9 @@ #include "trace.h" #include CONFIG_DEVICES -GlobalProperty pc_compat_5_2[] = {}; +GlobalProperty pc_compat_5_2[] = { + { "ICH9-LPC", "x-smi-cpu-hotunplug", "off" }, +}; const size_t pc_compat_5_2_len = G_N_ELEMENTS(pc_compat_5_2); GlobalProperty pc_compat_5_1[] = { diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index da80430144..d3145bf014 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -392,6 +392,12 @@ static void smi_features_ok_callback(void *opaque) return; } + if (guest_cpu_hotplug_features == + BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)) { + /* cpu hot-unplug is unsupported without cpu-hotplug */ + return; + } + /* valid feature subset requested, lock it down, report success */ lpc->smi_negotiated_features = guest_features; lpc->smi_features_ok = 1; @@ -774,7 +780,7 @@ static Property ich9_lpc_properties[] = { DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features, ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT, true), DEFINE_PROP_BIT64("x-smi-cpu-hotunplug", ICH9LPCState, smi_host_features, - ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT, false), + ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT, true), DEFINE_PROP_END_OF_LIST(), }; From 8ad4e4519c6d0981bd511f7ad9107c16c7a94f5c Mon Sep 17 00:00:00 2001 From: Zenghui Yu Date: Fri, 4 Dec 2020 11:09:53 +0800 Subject: [PATCH 63/65] pcie_aer: Fix help message of pcie_aer_inject_error command There is an interesting typo in the help message of pcie_aer_inject_error command. Use 'tlp' instead of 'tlb' to match the PCIe AER term. Signed-off-by: Zenghui Yu Message-Id: <20201204030953.837-1-yuzenghui@huawei.com> Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hmp-commands.hx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index ff2d7aa8f3..dd460eb908 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1302,8 +1302,8 @@ ERST " -c for correctable error\n\t\t\t" " = qdev device id\n\t\t\t" " = error string or 32bit\n\t\t\t" - " = 32bit x 4\n\t\t\t" - " = 32bit x 4", + " = 32bit x 4\n\t\t\t" + " = 32bit x 4", .cmd = hmp_pcie_aer_inject_error, }, From 06e97442420b03a1e0ff05e8eb554fac684ca736 Mon Sep 17 00:00:00 2001 From: Andrew Melnychenko Date: Thu, 3 Dec 2020 13:07:12 +0200 Subject: [PATCH 64/65] hw/virtio-pci Added counter for pcie capabilities offsets. Removed hardcoded offset for ats. Added cap offset counter for future capabilities like AER. Signed-off-by: Andrew Melnychenko Message-Id: <20201203110713.204938-2-andrew@daynix.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 36524a5728..ceaa233129 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1798,6 +1798,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) if (pcie_port && pci_is_express(pci_dev)) { int pos; + uint16_t last_pcie_cap_offset = PCI_CONFIG_SPACE_SIZE; pos = pcie_endpoint_cap_init(pci_dev, 0); assert(pos > 0); @@ -1833,7 +1834,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) } if (proxy->flags & VIRTIO_PCI_FLAG_ATS) { - pcie_ats_init(pci_dev, 256); + pcie_ats_init(pci_dev, last_pcie_cap_offset); + last_pcie_cap_offset += PCI_EXT_CAP_ATS_SIZEOF; } if (proxy->flags & VIRTIO_PCI_FLAG_INIT_FLR) { From fdfa3b1d6f9edd97c807df496a0d8e9ea49240da Mon Sep 17 00:00:00 2001 From: Andrew Melnychenko Date: Thu, 3 Dec 2020 13:07:13 +0200 Subject: [PATCH 65/65] hw/virtio-pci Added AER capability. Added AER capability for virtio-pci devices. Also added property for devices, by default AER is disabled. Signed-off-by: Andrew Melnychenko Message-Id: <20201203110713.204938-3-andrew@daynix.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.c | 16 ++++++++++++++++ hw/virtio/virtio-pci.h | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ceaa233129..f863f69ede 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1817,6 +1817,12 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) */ pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3); + if (proxy->flags & VIRTIO_PCI_FLAG_AER) { + pcie_aer_init(pci_dev, PCI_ERR_VER, last_pcie_cap_offset, + PCI_ERR_SIZEOF, NULL); + last_pcie_cap_offset += PCI_ERR_SIZEOF; + } + if (proxy->flags & VIRTIO_PCI_FLAG_INIT_DEVERR) { /* Init error enabling flags */ pcie_cap_deverr_init(pci_dev); @@ -1858,7 +1864,15 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) static void virtio_pci_exit(PCIDevice *pci_dev) { + VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); + bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) && + !pci_bus_is_root(pci_get_bus(pci_dev)); + msix_uninit_exclusive_bar(pci_dev); + if (proxy->flags & VIRTIO_PCI_FLAG_AER && pcie_port && + pci_is_express(pci_dev)) { + pcie_aer_exit(pci_dev); + } } static void virtio_pci_reset(DeviceState *qdev) @@ -1911,6 +1925,8 @@ static Property virtio_pci_properties[] = { VIRTIO_PCI_FLAG_INIT_PM_BIT, true), DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_INIT_FLR_BIT, true), + DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags, + VIRTIO_PCI_FLAG_AER_BIT, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 06e2af12de..d7d5d403a9 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -41,6 +41,7 @@ enum { VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, VIRTIO_PCI_FLAG_INIT_PM_BIT, VIRTIO_PCI_FLAG_INIT_FLR_BIT, + VIRTIO_PCI_FLAG_AER_BIT, }; /* Need to activate work-arounds for buggy guests at vmstate load. */ @@ -80,6 +81,9 @@ enum { /* Init Function Level Reset capability */ #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT) +/* Advanced Error Reporting capability */ +#define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT) + typedef struct { MSIMessage msg; int virq;