From 3830df5f83b9b52d9496763ce1a50afb9231c998 Mon Sep 17 00:00:00 2001 From: nia Date: Fri, 1 Oct 2021 15:30:03 +0000 Subject: [PATCH 1/8] configure: Loosen GCC requirement from 7.5.0 to 7.4.0 As discussed in issue 614, we're shipping GCC 7.4.0 as the system compiler in NetBSD 9, the most recent stable branch, and are still actively interested in QEMU on this platform. The differences between GCC 7.5.0 and 7.4.0 are trivial. Signed-off-by: Nia Alarie Reviewed-by: Richard Henderson Message-Id: Signed-off-by: Paolo Bonzini --- configure | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 1043ccce4f..b0b1a1cc25 100755 --- a/configure +++ b/configure @@ -2094,8 +2094,8 @@ cat > $TMPC << EOF # endif # endif #elif defined(__GNUC__) && defined(__GNUC_MINOR__) -# if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 5) -# error You need at least GCC v7.5.0 to compile QEMU +# if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 4) +# error You need at least GCC v7.4.0 to compile QEMU # endif #else # error You either need GCC or Clang to compiler QEMU @@ -2103,7 +2103,7 @@ cat > $TMPC << EOF int main (void) { return 0; } EOF if ! compile_prog "" "" ; then - error_exit "You need at least GCC v7.5 or Clang v6.0 (or XCode Clang v10.0)" + error_exit "You need at least GCC v7.4 or Clang v6.0 (or XCode Clang v10.0)" fi # Accumulate -Wfoo and -Wno-bar separately. From 75b98cb9f6456ccf194211beffcbf93b0a995fa4 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 29 Sep 2021 18:24:43 +0200 Subject: [PATCH 2/8] virtio-mem-pci: Fix memory leak when creating MEMORY_DEVICE_SIZE_CHANGE event Apparently, we don't have to duplicate the string. Fixes: 722a3c783ef4 ("virtio-pci: Send qapi events when the virtio-mem size changes") Cc: qemu-stable@nongnu.org Signed-off-by: David Hildenbrand Reviewed-by: Markus Armbruster Message-Id: <20210929162445.64060-2-david@redhat.com> Signed-off-by: Paolo Bonzini --- hw/virtio/virtio-mem-pci.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c index fa5395cd88..7e384b7397 100644 --- a/hw/virtio/virtio-mem-pci.c +++ b/hw/virtio/virtio-mem-pci.c @@ -88,13 +88,8 @@ static void virtio_mem_pci_size_change_notify(Notifier *notifier, void *data) size_change_notifier); DeviceState *dev = DEVICE(pci_mem); const uint64_t * const size_p = data; - const char *id = NULL; - if (dev->id) { - id = g_strdup(dev->id); - } - - qapi_event_send_memory_device_size_change(!!id, id, *size_p); + qapi_event_send_memory_device_size_change(!!dev->id, dev->id, *size_p); } static void virtio_mem_pci_class_init(ObjectClass *klass, void *data) From d89dd28f0e29c9eae997b0cd645208454a2f3374 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 29 Sep 2021 18:24:44 +0200 Subject: [PATCH 3/8] qapi: Include qom-path in MEMORY_DEVICE_SIZE_CHANGE qapi events As we might not always have a device id, it is impossible to always match MEMORY_DEVICE_SIZE_CHANGE events to an actual device. Let's include the qom-path in the event, which allows for reliable mapping of events to devices. Fixes: 722a3c783ef4 ("virtio-pci: Send qapi events when the virtio-mem size changes") Suggested-by: Markus Armbruster Reviewed-by: Markus Armbruster Signed-off-by: David Hildenbrand Message-Id: <20210929162445.64060-3-david@redhat.com> Signed-off-by: Paolo Bonzini --- hw/virtio/virtio-mem-pci.c | 5 ++++- qapi/machine.json | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c index 7e384b7397..be2383b0c5 100644 --- a/hw/virtio/virtio-mem-pci.c +++ b/hw/virtio/virtio-mem-pci.c @@ -87,9 +87,12 @@ static void virtio_mem_pci_size_change_notify(Notifier *notifier, void *data) VirtIOMEMPCI *pci_mem = container_of(notifier, VirtIOMEMPCI, size_change_notifier); DeviceState *dev = DEVICE(pci_mem); + char *qom_path = object_get_canonical_path(OBJECT(dev)); const uint64_t * const size_p = data; - qapi_event_send_memory_device_size_change(!!dev->id, dev->id, *size_p); + qapi_event_send_memory_device_size_change(!!dev->id, dev->id, *size_p, + qom_path); + g_free(qom_path); } static void virtio_mem_pci_class_init(ObjectClass *klass, void *data) diff --git a/qapi/machine.json b/qapi/machine.json index 0e91a57a76..5db54df298 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1336,8 +1336,11 @@ # action). # # @id: device's ID +# # @size: the new size of memory that the device provides # +# @qom-path: path to the device object in the QOM tree (since 6.2) +# # Note: this event is rate-limited. # # Since: 5.1 @@ -1350,7 +1353,7 @@ # ## { 'event': 'MEMORY_DEVICE_SIZE_CHANGE', - 'data': { '*id': 'str', 'size': 'size' } } + 'data': { '*id': 'str', 'size': 'size', 'qom-path' : 'str'} } ## From 77ae2302ae167aa840d5a3aa489f7958db7c1426 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 29 Sep 2021 18:24:45 +0200 Subject: [PATCH 4/8] monitor: Rate-limit MEMORY_DEVICE_SIZE_CHANGE qapi events per device We want to rate-limit MEMORY_DEVICE_SIZE_CHANGE events per device, otherwise we can lose some events for devices. We can now use the qom-path to reliably map an event to a device and make rate-limiting device-aware. This was noticed by starting a VM with two virtio-mem devices that each have a requested size > 0. The Linux guest will initialize both devices in parallel, resulting in losing MEMORY_DEVICE_SIZE_CHANGE events for one of the devices. Fixes: 722a3c783ef4 ("virtio-pci: Send qapi events when the virtio-mem size changes") Suggested-by: Markus Armbruster Reviewed-by: Markus Armbruster Signed-off-by: David Hildenbrand Message-Id: <20210929162445.64060-4-david@redhat.com> Signed-off-by: Paolo Bonzini --- monitor/monitor.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/monitor/monitor.c b/monitor/monitor.c index 46a171bca6..21c7a68758 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -474,6 +474,10 @@ static unsigned int qapi_event_throttle_hash(const void *key) hash += g_str_hash(qdict_get_str(evstate->data, "node-name")); } + if (evstate->event == QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE) { + hash += g_str_hash(qdict_get_str(evstate->data, "qom-path")); + } + return hash; } @@ -496,6 +500,11 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b) qdict_get_str(evb->data, "node-name")); } + if (eva->event == QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE) { + return !strcmp(qdict_get_str(eva->data, "qom-path"), + qdict_get_str(evb->data, "qom-path")); + } + return TRUE; } From 45e576c74533c70b38ba00f0c298dcdbc1635163 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Tue, 27 Jul 2021 10:25:42 +0200 Subject: [PATCH 5/8] tpm: mark correct memory region range dirty when clearing RAM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We might not start at the beginning of the memory region. Let's calculate the offset into the memory region via the difference in the host addresses. Acked-by: Stefan Berger Fixes: ffab1be70692 ("tpm: clear RAM when "memory overwrite" requested") Cc: Marc-André Lureau Cc: Paolo Bonzini Cc: "Michael S. Tsirkin" Cc: Eduardo Habkost Cc: Alex Williamson Cc: Dr. David Alan Gilbert Cc: Igor Mammedov Cc: Claudio Fontana Cc: Thomas Huth Cc: "Alex Bennée" Cc: Peter Xu Cc: Laurent Vivier Cc: Stefan Berger Signed-off-by: David Hildenbrand Reviewed-by: Peter Xu Message-Id: <20210727082545.17934-2-david@redhat.com> Signed-off-by: Paolo Bonzini --- hw/tpm/tpm_ppi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c index 362edcc5c9..274e9aa4b0 100644 --- a/hw/tpm/tpm_ppi.c +++ b/hw/tpm/tpm_ppi.c @@ -30,11 +30,14 @@ void tpm_ppi_reset(TPMPPI *tpmppi) guest_phys_blocks_init(&guest_phys_blocks); guest_phys_blocks_append(&guest_phys_blocks); QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) { + hwaddr mr_offs = block->host_addr - + (uint8_t *)memory_region_get_ram_ptr(block->mr); + trace_tpm_ppi_memset(block->host_addr, block->target_end - block->target_start); memset(block->host_addr, 0, block->target_end - block->target_start); - memory_region_set_dirty(block->mr, 0, + memory_region_set_dirty(block->mr, mr_offs, block->target_end - block->target_start); } guest_phys_blocks_free(&guest_phys_blocks); From 602f8ea79ce39b7bd6d2e22c686ef05227e1876b Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Tue, 27 Jul 2021 10:25:43 +0200 Subject: [PATCH 6/8] softmmu/memory_mapping: never merge ranges accross memory regions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's make sure to not merge when different memory regions are involved. Unlikely, but theoretically possible. Acked-by: Stefan Berger Reviewed-by: Peter Xu Cc: Marc-André Lureau Cc: Paolo Bonzini Cc: "Michael S. Tsirkin" Cc: Eduardo Habkost Cc: Alex Williamson Cc: Dr. David Alan Gilbert Cc: Igor Mammedov Cc: Claudio Fontana Cc: Thomas Huth Cc: "Alex Bennée" Cc: Peter Xu Cc: Laurent Vivier Cc: Stefan Berger Signed-off-by: David Hildenbrand Message-Id: <20210727082545.17934-3-david@redhat.com> Signed-off-by: Paolo Bonzini --- softmmu/memory_mapping.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c index e7af276546..d401ca7e31 100644 --- a/softmmu/memory_mapping.c +++ b/softmmu/memory_mapping.c @@ -229,7 +229,8 @@ static void guest_phys_blocks_region_add(MemoryListener *listener, /* we want continuity in both guest-physical and host-virtual memory */ if (predecessor->target_end < target_start || - predecessor->host_addr + predecessor_size != host_addr) { + predecessor->host_addr + predecessor_size != host_addr || + predecessor->mr != section->mr) { predecessor = NULL; } } From 3513bb1be1f025e011a69bafd02b6f59fa1d8383 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Tue, 27 Jul 2021 10:25:44 +0200 Subject: [PATCH 7/8] softmmu/memory_mapping: factor out adding physical memory ranges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's factor out adding a MemoryRegionSection to the list, to be reused in RamDiscardManager context next. Reviewed-by: Stefan Berger Reviewed-by: Peter Xu Cc: Marc-André Lureau Cc: Paolo Bonzini Cc: "Michael S. Tsirkin" Cc: Eduardo Habkost Cc: Alex Williamson Cc: Dr. David Alan Gilbert Cc: Igor Mammedov Cc: Claudio Fontana Cc: Thomas Huth Cc: "Alex Bennée" Cc: Peter Xu Cc: Laurent Vivier Cc: Stefan Berger Signed-off-by: David Hildenbrand Message-Id: <20210727082545.17934-4-david@redhat.com> Signed-off-by: Paolo Bonzini --- softmmu/memory_mapping.c | 41 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c index d401ca7e31..a2af02c41c 100644 --- a/softmmu/memory_mapping.c +++ b/softmmu/memory_mapping.c @@ -193,29 +193,14 @@ typedef struct GuestPhysListener { MemoryListener listener; } GuestPhysListener; -static void guest_phys_blocks_region_add(MemoryListener *listener, +static void guest_phys_block_add_section(GuestPhysListener *g, MemoryRegionSection *section) { - GuestPhysListener *g; - uint64_t section_size; - hwaddr target_start, target_end; - uint8_t *host_addr; - GuestPhysBlock *predecessor; - - /* we only care about RAM */ - if (!memory_region_is_ram(section->mr) || - memory_region_is_ram_device(section->mr) || - memory_region_is_nonvolatile(section->mr)) { - return; - } - - g = container_of(listener, GuestPhysListener, listener); - section_size = int128_get64(section->size); - target_start = section->offset_within_address_space; - target_end = target_start + section_size; - host_addr = memory_region_get_ram_ptr(section->mr) + - section->offset_within_region; - predecessor = NULL; + const hwaddr target_start = section->offset_within_address_space; + const hwaddr target_end = target_start + int128_get64(section->size); + uint8_t *host_addr = memory_region_get_ram_ptr(section->mr) + + section->offset_within_region; + GuestPhysBlock *predecessor = NULL; /* find continuity in guest physical address space */ if (!QTAILQ_EMPTY(&g->list->head)) { @@ -261,6 +246,20 @@ static void guest_phys_blocks_region_add(MemoryListener *listener, #endif } +static void guest_phys_blocks_region_add(MemoryListener *listener, + MemoryRegionSection *section) +{ + GuestPhysListener *g = container_of(listener, GuestPhysListener, listener); + + /* we only care about RAM */ + if (!memory_region_is_ram(section->mr) || + memory_region_is_ram_device(section->mr) || + memory_region_is_nonvolatile(section->mr)) { + return; + } + guest_phys_block_add_section(g, section); +} + void guest_phys_blocks_append(GuestPhysBlockList *list) { GuestPhysListener g = { 0 }; From cb83ba8c1ab856b4327e7e869c410bbfd4152c2c Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Tue, 27 Jul 2021 10:25:45 +0200 Subject: [PATCH 8/8] softmmu/memory_mapping: optimize for RamDiscardManager sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit virtio-mem logically plugs/unplugs memory within a sparse memory region and notifies via the RamDiscardManager interface when parts become plugged (populated) or unplugged (discarded). Currently, we end up (via the two users) 1) zeroing all logically unplugged/discarded memory during TPM resets. 2) reading all logically unplugged/discarded memory when dumping, to figure out the content is zero. 1) is always bad, because we assume unplugged memory stays discarded (and is already implicitly zero). 2) isn't that bad with anonymous memory, we end up reading the zero page (slow and unnecessary, though). However, once we use some file-backed memory (future use case), even reading will populate memory. Let's cut out all parts marked as not-populated (discarded) via the RamDiscardManager. As virtio-mem is the single user, this now means that logically unplugged memory ranges will no longer be included in the dump, which results in smaller dump files and faster dumping. virtio-mem has a minimum granularity of 1 MiB (and the default is usually 2 MiB). Theoretically, we can see quite some fragmentation, in practice we won't have it completely fragmented in 1 MiB pieces. Still, we might end up with many physical ranges. Both, the ELF format and kdump seem to be ready to support many individual ranges (e.g., for ELF it seems to be UINT32_MAX, kdump has a linear bitmap). Reviewed-by: Peter Xu Cc: Marc-André Lureau Cc: Paolo Bonzini Cc: "Michael S. Tsirkin" Cc: Eduardo Habkost Cc: Alex Williamson Cc: Dr. David Alan Gilbert Cc: Igor Mammedov Cc: Claudio Fontana Cc: Thomas Huth Cc: "Alex Bennée" Cc: Peter Xu Cc: Laurent Vivier Cc: Stefan Berger Signed-off-by: David Hildenbrand Message-Id: <20210727082545.17934-5-david@redhat.com> Signed-off-by: Paolo Bonzini --- softmmu/memory_mapping.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c index a2af02c41c..a62eaa49cc 100644 --- a/softmmu/memory_mapping.c +++ b/softmmu/memory_mapping.c @@ -246,6 +246,15 @@ static void guest_phys_block_add_section(GuestPhysListener *g, #endif } +static int guest_phys_ram_populate_cb(MemoryRegionSection *section, + void *opaque) +{ + GuestPhysListener *g = opaque; + + guest_phys_block_add_section(g, section); + return 0; +} + static void guest_phys_blocks_region_add(MemoryListener *listener, MemoryRegionSection *section) { @@ -257,6 +266,17 @@ static void guest_phys_blocks_region_add(MemoryListener *listener, memory_region_is_nonvolatile(section->mr)) { return; } + + /* for special sparse regions, only add populated parts */ + if (memory_region_has_ram_discard_manager(section->mr)) { + RamDiscardManager *rdm; + + rdm = memory_region_get_ram_discard_manager(section->mr); + ram_discard_manager_replay_populated(rdm, section, + guest_phys_ram_populate_cb, g); + return; + } + guest_phys_block_add_section(g, section); }