From 11118c7236a1279e2d2afa86089dd365508d3d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 5 Sep 2023 13:10:59 +0200 Subject: [PATCH 01/43] hw/i386: Rename kvmvapic.c -> vapic.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vAPIC isn't KVM specific, so having its name prefixed 'kvm' is misleading. Rename it simply 'vapic'. Rename the single function prefixed 'kvm'. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20230905145159.7898-1-philmd@linaro.org> --- hw/i386/meson.build | 2 +- hw/i386/{kvmvapic.c => vapic.c} | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) rename hw/i386/{kvmvapic.c => vapic.c} (99%) diff --git a/hw/i386/meson.build b/hw/i386/meson.build index b9c1ca39cb..d8b70ef3e9 100644 --- a/hw/i386/meson.build +++ b/hw/i386/meson.build @@ -1,7 +1,7 @@ i386_ss = ss.source_set() i386_ss.add(files( 'fw_cfg.c', - 'kvmvapic.c', + 'vapic.c', 'e820_memory_layout.c', 'multiboot.c', 'x86.c', diff --git a/hw/i386/kvmvapic.c b/hw/i386/vapic.c similarity index 99% rename from hw/i386/kvmvapic.c rename to hw/i386/vapic.c index 61a65ef2ab..f5b1db7e5f 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/vapic.c @@ -747,8 +747,7 @@ static void do_vapic_enable(CPUState *cs, run_on_cpu_data data) s->state = VAPIC_ACTIVE; } -static void kvmvapic_vm_state_change(void *opaque, bool running, - RunState state) +static void vapic_vm_state_change(void *opaque, bool running, RunState state) { MachineState *ms = MACHINE(qdev_get_machine()); VAPICROMState *s = opaque; @@ -793,7 +792,7 @@ static int vapic_post_load(void *opaque, int version_id) if (!s->vmsentry) { s->vmsentry = - qemu_add_vm_change_state_handler(kvmvapic_vm_state_change, s); + qemu_add_vm_change_state_handler(vapic_vm_state_change, s); } return 0; } From 261bbc3b30973d9adfd46f0d2e315d9dad1707c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 10 Nov 2023 22:04:53 +0000 Subject: [PATCH 02/43] sysemu/xen: Forbid using Xen headers in user emulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Xen is a system specific accelerator, it makes no sense to include its headers in user emulation. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-3-philmd@linaro.org> --- include/sysemu/xen.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h index bc13ad5692..a9f591f26d 100644 --- a/include/sysemu/xen.h +++ b/include/sysemu/xen.h @@ -10,6 +10,10 @@ #ifndef SYSEMU_XEN_H #define SYSEMU_XEN_H +#ifdef CONFIG_USER_ONLY +#error Cannot include sysemu/xen.h from user emulation +#endif + #include "exec/cpu-common.h" #ifdef NEED_CPU_H @@ -26,16 +30,13 @@ extern bool xen_allowed; #define xen_enabled() (xen_allowed) -#ifndef CONFIG_USER_ONLY void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length); void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, struct MemoryRegion *mr, Error **errp); -#endif #else /* !CONFIG_XEN_IS_POSSIBLE */ #define xen_enabled() 0 -#ifndef CONFIG_USER_ONLY static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length) { /* nothing */ @@ -45,7 +46,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, { g_assert_not_reached(); } -#endif #endif /* CONFIG_XEN_IS_POSSIBLE */ From 3e5e5d479eaa1e3ea8535db3ee44eec147f2f1aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 13 Nov 2023 13:28:38 +0100 Subject: [PATCH 03/43] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic version of CONFIG_XEN accelerator. Use it in order to use "sysemu/xen-mapcache.h" in target-agnostic files. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-4-philmd@linaro.org> --- include/sysemu/xen-mapcache.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h index c8e7c2f6cf..10c2e3082a 100644 --- a/include/sysemu/xen-mapcache.h +++ b/include/sysemu/xen-mapcache.h @@ -10,10 +10,11 @@ #define XEN_MAPCACHE_H #include "exec/cpu-common.h" +#include "sysemu/xen.h" typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset, ram_addr_t size); -#ifdef CONFIG_XEN +#ifdef CONFIG_XEN_IS_POSSIBLE void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque); From 5d5bb9c8fd607af542a53c0db1ddfb5cce0a4e6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 10 Nov 2023 22:37:20 +0100 Subject: [PATCH 04/43] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit physmem.c doesn't use any declaration from "hw/xen/xen.h", it only requires "sysemu/xen.h" and "system/xen-mapcache.h". Suggested-by: David Woodhouse Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Reviewed-by: David Hildenbrand Message-Id: <20231114143816.71079-5-philmd@linaro.org> --- system/physmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/physmem.c b/system/physmem.c index 3adda08ebf..6e9ed97597 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -35,7 +35,7 @@ #include "hw/qdev-core.h" #include "hw/qdev-properties.h" #include "hw/boards.h" -#include "hw/xen/xen.h" +#include "sysemu/xen.h" #include "sysemu/kvm.h" #include "sysemu/tcg.h" #include "sysemu/qtest.h" From 06c8337653cbcfab0801efaababbbbaa7bb7eaf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 11:29:35 +0100 Subject: [PATCH 05/43] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similarly to the restriction in hw/pci/msix.c (see commit e1e4bf2252 "msix: fix msix_vector_masked"), restrict the xen_is_pirq_msi() call in msi_is_masked() to Xen. No functional change intended. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-7-philmd@linaro.org> --- hw/pci/msi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/pci/msi.c b/hw/pci/msi.c index 041b0bdbec..8104ac1d91 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -23,6 +23,7 @@ #include "hw/xen/xen.h" #include "qemu/range.h" #include "qapi/error.h" +#include "sysemu/xen.h" #include "hw/i386/kvm/xen_evtchn.h" @@ -308,7 +309,7 @@ bool msi_is_masked(const PCIDevice *dev, unsigned int vector) } data = pci_get_word(dev->config + msi_data_off(dev, msi64bit)); - if (xen_is_pirq_msi(data)) { + if (xen_enabled() && xen_is_pirq_msi(data)) { return false; } From 9cd909ac3505c3561570793753c70ceafdd32eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 11:44:31 +0100 Subject: [PATCH 06/43] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 04b0de0ee8 ("xen: factor out common functions") xen_hvm_inject_msi() stub is not required. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-8-philmd@linaro.org> --- stubs/xen-hw-stub.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c index 7d7ffe83a9..6cf0e9a4c1 100644 --- a/stubs/xen-hw-stub.c +++ b/stubs/xen-hw-stub.c @@ -24,10 +24,6 @@ int xen_set_pci_link_route(uint8_t link, uint8_t irq) return -1; } -void xen_hvm_inject_msi(uint64_t addr, uint32_t data) -{ -} - int xen_is_pirq_msi(uint32_t msi_data) { return 0; From b934c3fa21dbe23a17c9fdeccb85fb46418bc699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 11:22:29 +0100 Subject: [PATCH 07/43] hw/xen: Rename 'ram_memory' global variable as 'xen_memory' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid a potential global variable shadow in hw/i386/pc_piix.c::pc_init1(), rename Xen's "ram_memory" as "xen_memory". Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-11-philmd@linaro.org> --- hw/arm/xen_arm.c | 6 +++--- hw/i386/xen/xen-hvm.c | 10 +++++----- hw/xen/xen-hvm-common.c | 6 +++--- include/hw/xen/xen-hvm-common.h | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c index 32776d94df..15fa7dfa84 100644 --- a/hw/arm/xen_arm.c +++ b/hw/arm/xen_arm.c @@ -114,14 +114,14 @@ static void xen_init_ram(MachineState *machine) block_len = GUEST_RAM1_BASE + ram_size[1]; } - memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len, + memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len, &error_fatal); - memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &ram_memory, + memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &xen_memory, GUEST_RAM0_BASE, ram_size[0]); memory_region_add_subregion(sysmem, GUEST_RAM0_BASE, &ram_lo); if (ram_size[1] > 0) { - memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &ram_memory, + memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &xen_memory, GUEST_RAM1_BASE, ram_size[1]); memory_region_add_subregion(sysmem, GUEST_RAM1_BASE, &ram_hi); } diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index f42621e674..1ae943370b 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -149,12 +149,12 @@ static void xen_ram_init(PCMachineState *pcms, */ block_len = (4 * GiB) + x86ms->above_4g_mem_size; } - memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len, + memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len, &error_fatal); - *ram_memory_p = &ram_memory; + *ram_memory_p = &xen_memory; memory_region_init_alias(&ram_640k, NULL, "xen.ram.640k", - &ram_memory, 0, 0xa0000); + &xen_memory, 0, 0xa0000); memory_region_add_subregion(sysmem, 0, &ram_640k); /* Skip of the VGA IO memory space, it will be registered later by the VGA * emulated device. @@ -163,12 +163,12 @@ static void xen_ram_init(PCMachineState *pcms, * the Options ROM, so it is registered here as RAM. */ memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", - &ram_memory, 0xc0000, + &xen_memory, 0xc0000, x86ms->below_4g_mem_size - 0xc0000); memory_region_add_subregion(sysmem, 0xc0000, &ram_lo); if (x86ms->above_4g_mem_size > 0) { memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", - &ram_memory, 0x100000000ULL, + &xen_memory, 0x100000000ULL, x86ms->above_4g_mem_size); memory_region_add_subregion(sysmem, 0x100000000ULL, &ram_hi); } diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index baa1adb9f2..dc69cada57 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -9,7 +9,7 @@ #include "hw/boards.h" #include "hw/xen/arch_hvm.h" -MemoryRegion ram_memory; +MemoryRegion xen_memory; void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr, Error **errp) @@ -26,7 +26,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr, return; } - if (mr == &ram_memory) { + if (mr == &xen_memory) { return; } @@ -53,7 +53,7 @@ static void xen_set_memory(struct MemoryListener *listener, { XenIOState *state = container_of(listener, XenIOState, memory_listener); - if (section->mr == &ram_memory) { + if (section->mr == &xen_memory) { return; } else { if (add) { diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h index 4b1d728f35..65a51aac2e 100644 --- a/include/hw/xen/xen-hvm-common.h +++ b/include/hw/xen/xen-hvm-common.h @@ -15,7 +15,7 @@ #include "qemu/error-report.h" #include -extern MemoryRegion ram_memory; +extern MemoryRegion xen_memory; extern MemoryListener xen_io_listener; extern DeviceListener xen_device_listener; From 8570951fcca78fcb02807153b07c93fd5c22cde7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 10 Nov 2023 22:36:37 +0100 Subject: [PATCH 08/43] hw/xen: Use target-agnostic qemu_target_page_bits() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of the target-specific TARGET_PAGE_BITS definition, use qemu_target_page_bits() which is target agnostic. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-15-philmd@linaro.org> --- hw/xen/xen-hvm-common.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index dc69cada57..1627da7398 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -1,6 +1,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" #include "qapi/error.h" +#include "exec/target_page.h" #include "trace.h" #include "hw/pci/pci_host.h" @@ -14,6 +15,7 @@ MemoryRegion xen_memory; void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr, Error **errp) { + unsigned target_page_bits = qemu_target_page_bits(); unsigned long nr_pfn; xen_pfn_t *pfn_list; int i; @@ -32,11 +34,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr, trace_xen_ram_alloc(ram_addr, size); - nr_pfn = size >> TARGET_PAGE_BITS; + nr_pfn = size >> target_page_bits; pfn_list = g_new(xen_pfn_t, nr_pfn); for (i = 0; i < nr_pfn; i++) { - pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i; + pfn_list[i] = (ram_addr >> target_page_bits) + i; } if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, pfn_list)) { From 92dfc8a25785c3df47bdf2788be3d229e6185f62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 07:42:21 +0100 Subject: [PATCH 09/43] hw/xen/xen_pt: Add missing license MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit eaab4d60d3 ("Introduce Xen PCI Passthrough, qdevice") introduced both xen_pt.[ch], but only added the license to xen_pt.c. Use the same license for xen_pt.h. Suggested-by: David Woodhouse Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-17-philmd@linaro.org> --- hw/xen/xen_pt.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 31bcfdf705..d3180bb134 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -1,3 +1,13 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * Copyright (c) 2007, Intel Corporation. + * + * SPDX-License-Identifier: GPL-2.0-only + * + * Alex Novik + * Allen Kay + * Guy Zana + */ #ifndef XEN_PT_H #define XEN_PT_H From f28b958cbf08c4019f99091208e5c877b857b030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 10 Nov 2023 22:50:35 +0100 Subject: [PATCH 10/43] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target specific. It also declares IGD methods, which are not target specific. Target-agnostic code can use IGD methods. To allow that, extract these methos into a new "hw/xen/xen_igd.h" header. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-18-philmd@linaro.org> --- accel/xen/xen-all.c | 1 + hw/i386/pc_piix.c | 1 + hw/xen/xen_pt.c | 3 ++- hw/xen/xen_pt.h | 14 -------------- hw/xen/xen_pt_config_init.c | 3 ++- hw/xen/xen_pt_graphics.c | 3 ++- hw/xen/xen_pt_stub.c | 2 +- include/hw/xen/xen_igd.h | 33 +++++++++++++++++++++++++++++++++ 8 files changed, 42 insertions(+), 18 deletions(-) create mode 100644 include/hw/xen/xen_igd.h diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c index 5ff0cb8bd9..0bdefce537 100644 --- a/accel/xen/xen-all.c +++ b/accel/xen/xen-all.c @@ -15,6 +15,7 @@ #include "hw/xen/xen_native.h" #include "hw/xen/xen-legacy-backend.h" #include "hw/xen/xen_pt.h" +#include "hw/xen/xen_igd.h" #include "chardev/char.h" #include "qemu/accel.h" #include "sysemu/cpus.h" diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ce6aad758d..e123458bbc 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -55,6 +55,7 @@ #ifdef CONFIG_XEN #include #include "hw/xen/xen_pt.h" +#include "hw/xen/xen_igd.h" #endif #include "hw/xen/xen-x86.h" #include "hw/xen/xen.h" diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 36e6f93c37..a8edabdabc 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -59,7 +59,8 @@ #include "hw/pci/pci.h" #include "hw/qdev-properties.h" #include "hw/qdev-properties-system.h" -#include "xen_pt.h" +#include "hw/xen/xen_pt.h" +#include "hw/xen/xen_igd.h" #include "hw/xen/xen.h" #include "hw/xen/xen-legacy-backend.h" #include "qemu/range.h" diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index d3180bb134..095a0f0365 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -15,9 +15,6 @@ #include "xen-host-pci-device.h" #include "qom/object.h" -bool xen_igd_gfx_pt_enabled(void); -void xen_igd_gfx_pt_set(bool value, Error **errp); - void xen_pt_log(const PCIDevice *d, const char *f, ...) G_GNUC_PRINTF(2, 3); #define XEN_PT_ERR(d, _f, _a...) xen_pt_log(d, "%s: Error: "_f, __func__, ##_a) @@ -62,12 +59,6 @@ typedef struct XenPTDeviceClass { XenPTQdevRealize pci_qdev_realize; } XenPTDeviceClass; -uint32_t igd_read_opregion(XenPCIPassthroughState *s); -void xen_igd_reserve_slot(PCIBus *pci_bus); -void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val); -void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s, - XenHostPCIDevice *dev); - /* function type for config reg */ typedef int (*xen_pt_conf_reg_init) (XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset, @@ -353,11 +344,6 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar) void *pci_assign_dev_load_option_rom(PCIDevice *dev, int *size, unsigned int domain, unsigned int bus, unsigned int slot, unsigned int function); -static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev) -{ - return (xen_igd_gfx_pt_enabled() - && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); -} int xen_pt_register_vga_regions(XenHostPCIDevice *dev); int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev); void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev, diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c index 2b8680b112..ba4cd78238 100644 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -15,7 +15,8 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/timer.h" -#include "xen_pt.h" +#include "hw/xen/xen_pt.h" +#include "hw/xen/xen_igd.h" #include "hw/xen/xen-legacy-backend.h" #define XEN_PT_MERGE_VALUE(value, data, val_mask) \ diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index 0aed3bb6fd..6c2e3f4840 100644 --- a/hw/xen/xen_pt_graphics.c +++ b/hw/xen/xen_pt_graphics.c @@ -3,7 +3,8 @@ */ #include "qemu/osdep.h" #include "qapi/error.h" -#include "xen_pt.h" +#include "hw/xen/xen_pt.h" +#include "hw/xen/xen_igd.h" #include "xen-host-pci-device.h" static unsigned long igd_guest_opregion; diff --git a/hw/xen/xen_pt_stub.c b/hw/xen/xen_pt_stub.c index 5c108446a8..72feebeb20 100644 --- a/hw/xen/xen_pt_stub.c +++ b/hw/xen/xen_pt_stub.c @@ -6,7 +6,7 @@ */ #include "qemu/osdep.h" -#include "hw/xen/xen_pt.h" +#include "hw/xen/xen_igd.h" #include "qapi/error.h" bool xen_igd_gfx_pt_enabled(void) diff --git a/include/hw/xen/xen_igd.h b/include/hw/xen/xen_igd.h new file mode 100644 index 0000000000..7ffca06c10 --- /dev/null +++ b/include/hw/xen/xen_igd.h @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * Copyright (c) 2007, Intel Corporation. + * + * SPDX-License-Identifier: GPL-2.0-only + * + * Alex Novik + * Allen Kay + * Guy Zana + */ +#ifndef XEN_IGD_H +#define XEN_IGD_H + +#include "hw/xen/xen-host-pci-device.h" + +typedef struct XenPCIPassthroughState XenPCIPassthroughState; + +bool xen_igd_gfx_pt_enabled(void); +void xen_igd_gfx_pt_set(bool value, Error **errp); + +uint32_t igd_read_opregion(XenPCIPassthroughState *s); +void xen_igd_reserve_slot(PCIBus *pci_bus); +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val); +void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s, + XenHostPCIDevice *dev); + +static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev) +{ + return (xen_igd_gfx_pt_enabled() + && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); +} + +#endif From 906c0876eef947b8abfe2da3ad15fd6c8fa1e2ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 15:23:24 +0100 Subject: [PATCH 11/43] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xen-hvm.c calls xc_set_hvm_param() from , so better compile it with Xen CPPFLAGS. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: David Woodhouse Message-Id: <20231114143816.71079-19-philmd@linaro.org> --- hw/i386/xen/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build index 3dc4c4f106..3f0df8bc07 100644 --- a/hw/i386/xen/meson.build +++ b/hw/i386/xen/meson.build @@ -1,8 +1,10 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files( - 'xen-hvm.c', 'xen_apic.c', 'xen_pvdevice.c', )) +i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files( + 'xen-hvm.c', +)) i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files( 'xen_platform.c', From 62d6cf9d63a35f7e0b6173c51b11a358f52a3419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 15:54:45 +0100 Subject: [PATCH 12/43] hw/xen/hvm: Inline TARGET_PAGE_ALIGN() macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use TARGET_PAGE_SIZE to calculate TARGET_PAGE_ALIGN. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Message-Id: <20231114163123.74888-2-philmd@linaro.org> --- hw/i386/xen/xen-hvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index 1ae943370b..8235782ef7 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -678,7 +678,7 @@ void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section, trace_xen_client_set_memory(start_addr, size, log_dirty); start_addr &= TARGET_PAGE_MASK; - size = TARGET_PAGE_ALIGN(size); + size = ROUND_UP(size, TARGET_PAGE_SIZE); if (add) { if (!memory_region_is_rom(section->mr)) { From 8ebb8682f6037f5373862d7ab2f72c2d3e387fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 15:53:26 +0100 Subject: [PATCH 13/43] hw/xen/hvm: Propagate page_mask to a pair of functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are going to replace TARGET_PAGE_MASK by a runtime variable. In order to reduce code duplication, propagate TARGET_PAGE_MASK to get_physmapping() and xen_phys_offset_to_gaddr(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Message-Id: <20231114163123.74888-3-philmd@linaro.org> --- hw/i386/xen/xen-hvm.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index 8235782ef7..844b11ae08 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -174,11 +174,12 @@ static void xen_ram_init(PCMachineState *pcms, } } -static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size) +static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size, + int page_mask) { XenPhysmap *physmap = NULL; - start_addr &= TARGET_PAGE_MASK; + start_addr &= page_mask; QLIST_FOREACH(physmap, &xen_physmap, list) { if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) { @@ -188,9 +189,10 @@ static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size) return NULL; } -static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size) +static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size, + int page_mask) { - hwaddr addr = phys_offset & TARGET_PAGE_MASK; + hwaddr addr = phys_offset & page_mask; XenPhysmap *physmap = NULL; QLIST_FOREACH(physmap, &xen_physmap, list) { @@ -252,7 +254,7 @@ static int xen_add_to_physmap(XenIOState *state, hwaddr phys_offset = memory_region_get_ram_addr(mr); const char *mr_name; - if (get_physmapping(start_addr, size)) { + if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) { return 0; } if (size <= 0) { @@ -325,7 +327,7 @@ static int xen_remove_from_physmap(XenIOState *state, XenPhysmap *physmap = NULL; hwaddr phys_offset = 0; - physmap = get_physmapping(start_addr, size); + physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK); if (physmap == NULL) { return -1; } @@ -373,7 +375,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state, int rc, i, j; const XenPhysmap *physmap = NULL; - physmap = get_physmapping(start_addr, size); + physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK); if (physmap == NULL) { /* not handled */ return; @@ -633,7 +635,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length) int rc; ram_addr_t start_pfn, nb_pages; - start = xen_phys_offset_to_gaddr(start, length); + start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK); if (length == 0) { length = TARGET_PAGE_SIZE; From 825f292d34a71e7f6d5cf592477ef28bde182758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 14 Nov 2023 15:58:23 +0100 Subject: [PATCH 14/43] hw/xen/hvm: Get target page size at runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to build this file once for all targets, replace: TARGET_PAGE_BITS -> qemu_target_page_bits() TARGET_PAGE_SIZE -> qemu_target_page_size() TARGET_PAGE_MASK -> -qemu_target_page_size() Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Message-Id: <20231114163123.74888-4-philmd@linaro.org> --- hw/i386/xen/xen-hvm.c | 62 +++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index 844b11ae08..7745cb3963 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -23,6 +23,7 @@ #include "hw/xen/xen-hvm-common.h" #include "hw/xen/arch_hvm.h" #include +#include "exec/target_page.h" static MemoryRegion ram_640k, ram_lo, ram_hi; static MemoryRegion *framebuffer; @@ -247,6 +248,9 @@ static int xen_add_to_physmap(XenIOState *state, MemoryRegion *mr, hwaddr offset_within_region) { + unsigned target_page_bits = qemu_target_page_bits(); + int page_size = qemu_target_page_size(); + int page_mask = -page_size; unsigned long nr_pages; int rc = 0; XenPhysmap *physmap = NULL; @@ -254,7 +258,7 @@ static int xen_add_to_physmap(XenIOState *state, hwaddr phys_offset = memory_region_get_ram_addr(mr); const char *mr_name; - if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) { + if (get_physmapping(start_addr, size, page_mask)) { return 0; } if (size <= 0) { @@ -294,9 +298,9 @@ go_physmap: return 0; } - pfn = phys_offset >> TARGET_PAGE_BITS; - start_gpfn = start_addr >> TARGET_PAGE_BITS; - nr_pages = size >> TARGET_PAGE_BITS; + pfn = phys_offset >> target_page_bits; + start_gpfn = start_addr >> target_page_bits; + nr_pages = size >> target_page_bits; rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn, start_gpfn); if (rc) { @@ -310,8 +314,8 @@ go_physmap: } rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid, - start_addr >> TARGET_PAGE_BITS, - (start_addr + size - 1) >> TARGET_PAGE_BITS, + start_addr >> target_page_bits, + (start_addr + size - 1) >> target_page_bits, XEN_DOMCTL_MEM_CACHEATTR_WB); if (rc) { error_report("pin_memory_cacheattr failed: %s", strerror(errno)); @@ -323,11 +327,14 @@ static int xen_remove_from_physmap(XenIOState *state, hwaddr start_addr, ram_addr_t size) { + unsigned target_page_bits = qemu_target_page_bits(); + int page_size = qemu_target_page_size(); + int page_mask = -page_size; int rc = 0; XenPhysmap *physmap = NULL; hwaddr phys_offset = 0; - physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK); + physmap = get_physmapping(start_addr, size, page_mask); if (physmap == NULL) { return -1; } @@ -338,9 +345,9 @@ static int xen_remove_from_physmap(XenIOState *state, DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at " "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset); - size >>= TARGET_PAGE_BITS; - start_addr >>= TARGET_PAGE_BITS; - phys_offset >>= TARGET_PAGE_BITS; + size >>= target_page_bits; + start_addr >>= target_page_bits; + phys_offset >>= target_page_bits; rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr, phys_offset); if (rc) { @@ -369,13 +376,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state, hwaddr start_addr, ram_addr_t size) { - hwaddr npages = size >> TARGET_PAGE_BITS; + unsigned target_page_bits = qemu_target_page_bits(); + int page_size = qemu_target_page_size(); + int page_mask = -page_size; + hwaddr npages = size >> target_page_bits; const int width = sizeof(unsigned long) * 8; size_t bitmap_size = DIV_ROUND_UP(npages, width); int rc, i, j; const XenPhysmap *physmap = NULL; - physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK); + physmap = get_physmapping(start_addr, size, page_mask); if (physmap == NULL) { /* not handled */ return; @@ -389,7 +399,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state, return; } - rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS, + rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits, npages, dirty_bitmap); if (rc < 0) { #ifndef ENODATA @@ -410,8 +420,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state, j = ctzl(map); map &= ~(1ul << j); memory_region_set_dirty(framebuffer, - (i * width + j) * TARGET_PAGE_SIZE, - TARGET_PAGE_SIZE); + (i * width + j) * page_size, page_size); }; } } @@ -631,17 +640,21 @@ void xen_register_framebuffer(MemoryRegion *mr) void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length) { + unsigned target_page_bits = qemu_target_page_bits(); + int page_size = qemu_target_page_size(); + int page_mask = -page_size; + if (unlikely(xen_in_migration)) { int rc; ram_addr_t start_pfn, nb_pages; - start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK); + start = xen_phys_offset_to_gaddr(start, length, page_mask); if (length == 0) { - length = TARGET_PAGE_SIZE; + length = page_size; } - start_pfn = start >> TARGET_PAGE_BITS; - nb_pages = ((start + length + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS) + start_pfn = start >> target_page_bits; + nb_pages = ((start + length + page_size - 1) >> target_page_bits) - start_pfn; rc = xen_modified_memory(xen_domid, start_pfn, nb_pages); if (rc) { @@ -664,6 +677,9 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp) void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section, bool add) { + unsigned target_page_bits = qemu_target_page_bits(); + int page_size = qemu_target_page_size(); + int page_mask = -page_size; hwaddr start_addr = section->offset_within_address_space; ram_addr_t size = int128_get64(section->size); bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA); @@ -679,8 +695,8 @@ void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section, trace_xen_client_set_memory(start_addr, size, log_dirty); - start_addr &= TARGET_PAGE_MASK; - size = ROUND_UP(size, TARGET_PAGE_SIZE); + start_addr &= page_mask; + size = ROUND_UP(size, page_size); if (add) { if (!memory_region_is_rom(section->mr)) { @@ -689,8 +705,8 @@ void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section, } else { mem_type = HVMMEM_ram_ro; if (xen_set_mem_type(xen_domid, mem_type, - start_addr >> TARGET_PAGE_BITS, - size >> TARGET_PAGE_BITS)) { + start_addr >> target_page_bits, + size >> target_page_bits)) { DPRINTF("xen_set_mem_type error, addr: "HWADDR_FMT_plx"\n", start_addr); } From 0a81424defeb1b59b8c5f7b40efb833a0867ca75 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 29 Feb 2024 00:37:21 +0800 Subject: [PATCH 15/43] hw/char/xen_console: Fix missing ERRP_GUARD() for error_prepend() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is the pointer of error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The xen_console_connect() passes @errp to error_prepend() without ERRP_GUARD(). There're 2 places will call xen_console_connect(): - xen_console_realize(): the @errp is from DeviceClass.realize()'s parameter. - xen_console_frontend_changed(): the @errp points its caller's @local_err. To avoid the issue like [1] said, add missing ERRP_GUARD() at the beginning of xen_console_connect(). [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Stefano Stabellini Cc: Anthony Perard Cc: Paul Durrant Cc: "Marc-André Lureau" Cc: Paolo Bonzini Signed-off-by: Zhao Liu Acked-by: Anthony PERARD Message-ID: <20240228163723.1775791-15-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/char/xen_console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c index 5cbee2f184..683c92aca1 100644 --- a/hw/char/xen_console.c +++ b/hw/char/xen_console.c @@ -206,6 +206,7 @@ static bool con_event(void *_xendev) static bool xen_console_connect(XenDevice *xendev, Error **errp) { + ERRP_GUARD(); XenConsole *con = XEN_CONSOLE_DEVICE(xendev); unsigned int port, limit; From 8538ceecd3a62c3233e15740cfb7071540547c6b Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 29 Feb 2024 22:38:59 +0800 Subject: [PATCH 16/43] hw/net/xen_nic: Fix missing ERRP_GUARD() for error_prepend() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is the pointer of error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The xen_netdev_connect() passes @errp to error_prepend(), and its @errp parameter is from xen_device_frontend_changed(). Though its @errp points to @local_err of xen_device_frontend_changed(), to follow the requirement of @errp, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Stefano Stabellini Cc: Anthony Perard Cc: Paul Durrant Cc: Jason Wang Signed-off-by: Zhao Liu Acked-by: Anthony PERARD Reviewed-by: Thomas Huth Message-ID: <20240229143914.1977550-3-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/net/xen_nic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c index 453fdb9819..89487b49ba 100644 --- a/hw/net/xen_nic.c +++ b/hw/net/xen_nic.c @@ -351,6 +351,7 @@ static bool net_event(void *_xendev) static bool xen_netdev_connect(XenDevice *xendev, Error **errp) { + ERRP_GUARD(); XenNetDev *netdev = XEN_NET_DEVICE(xendev); unsigned int port, rx_copy; From e15201171f9dc1e1aeb34bd08831de8af20925d6 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 29 Feb 2024 22:39:00 +0800 Subject: [PATCH 17/43] hw/remote/remote-obj: hw/misc/ivshmem: Fix missing ERRP_GUARD() for error_prepend() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is the pointer of error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The remote_object_set_fd() passes @errp to error_prepend(), and as a PropertyInfo.set method, its @errp is so widely sourced that it is necessary to protect it with ERRP_GUARD(). To avoid the issue like [1] said, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Elena Ufimtseva Cc: Jagannathan Raman Signed-off-by: Zhao Liu Reviewed-by: Thomas Huth Message-ID: <20240229143914.1977550-4-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/remote/remote-obj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c index 65b6f7cc86..dc27cc8da1 100644 --- a/hw/remote/remote-obj.c +++ b/hw/remote/remote-obj.c @@ -49,6 +49,7 @@ struct RemoteObject { static void remote_object_set_fd(Object *obj, const char *str, Error **errp) { + ERRP_GUARD(); RemoteObject *o = REMOTE_OBJECT(obj); int fd = -1; From f55cceac8c03e639711490f08996c32861591435 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 29 Feb 2024 22:39:13 +0800 Subject: [PATCH 18/43] target/i386/sev: Fix missing ERRP_GUARD() for error_prepend() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is the pointer of error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The sev_inject_launch_secret() passes @errp to error_prepend(), and as an APIs defined in target/i386/sev.h, it is necessary to protect its @errp with ERRP_GUARD(). To avoid the issue like [1] said, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Paolo Bonzini Cc: Marcelo Tosatti Signed-off-by: Zhao Liu Reviewed-by: Thomas Huth Message-ID: <20240229143914.1977550-17-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- target/i386/sev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/i386/sev.c b/target/i386/sev.c index 173de91afe..72930ff0dc 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -1044,6 +1044,7 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp) int sev_inject_launch_secret(const char *packet_hdr, const char *secret, uint64_t gpa, Error **errp) { + ERRP_GUARD(); struct kvm_sev_launch_secret input; g_autofree guchar *data = NULL, *hdr = NULL; int error, ret = 1; From 988b92f6d05500880f84fbcba861fc8c7ec1b5ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 28 Feb 2024 09:31:47 +0100 Subject: [PATCH 19/43] hw/i386/pc: Remove pc_compat_1_4..1.7[] left over declarations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These definitions were removed in commit ea985d235b ("pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7"). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20240301185936.95175-2-philmd@linaro.org> --- include/hw/i386/pc.h | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 5065590281..b958023187 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -288,18 +288,6 @@ extern const size_t pc_compat_2_1_len; extern GlobalProperty pc_compat_2_0[]; extern const size_t pc_compat_2_0_len; -extern GlobalProperty pc_compat_1_7[]; -extern const size_t pc_compat_1_7_len; - -extern GlobalProperty pc_compat_1_6[]; -extern const size_t pc_compat_1_6_len; - -extern GlobalProperty pc_compat_1_5[]; -extern const size_t pc_compat_1_5_len; - -extern GlobalProperty pc_compat_1_4[]; -extern const size_t pc_compat_1_4_len; - #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \ static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \ { \ From 07df0c3951a77b86027c2f6bd5b7a9273902d41f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 28 Feb 2024 12:09:07 +0100 Subject: [PATCH 20/43] hw/i386/pc: Use generated NotifyVmexitOption_str() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NotifyVmexitOption_str() is QAPI-generated in "qapi/qapi-types-run-state.h", which "sysemu/runstate.h" already includes. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20240301185936.95175-3-philmd@linaro.org> --- hw/i386/pc_piix.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index e123458bbc..ed777e3d61 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -61,6 +61,7 @@ #include "hw/xen/xen.h" #include "migration/global_state.h" #include "migration/misc.h" +#include "sysemu/runstate.h" #include "sysemu/numa.h" #include "hw/hyperv/vmbus-bridge.h" #include "hw/mem/nvdimm.h" @@ -383,9 +384,6 @@ static const QEnumLookup PCSouthBridgeOption_lookup = { .size = PC_SOUTH_BRIDGE_OPTION_MAX }; -#define NotifyVmexitOption_str(val) \ - qapi_enum_lookup(&NotifyVmexitOption_lookup, (val)) - static int pc_get_south_bridge(Object *obj, Error **errp) { PCMachineState *pcms = PC_MACHINE(obj); From 3ac5f6725a2158958afaf686d7a1ed3c7fbe0b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 28 Feb 2024 09:16:42 +0100 Subject: [PATCH 21/43] hw/i386/pc: Remove 'host_type' argument from pc_init1() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All callers use host_type=TYPE_I440FX_PCI_HOST_BRIDGE. Directly use this definition within pc_init1(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20240301185936.95175-4-philmd@linaro.org> --- hw/i386/pc_piix.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ed777e3d61..e14dce951d 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -101,8 +101,7 @@ static void piix_intx_routing_notifier_xen(PCIDevice *dev) } /* PC hardware initialisation */ -static void pc_init1(MachineState *machine, - const char *host_type, const char *pci_type) +static void pc_init1(MachineState *machine, const char *pci_type) { PCMachineState *pcms = PC_MACHINE(machine); PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); @@ -194,7 +193,7 @@ static void pc_init1(MachineState *machine, memory_region_init(pci_memory, NULL, "pci", UINT64_MAX); rom_memory = pci_memory; - phb = OBJECT(qdev_new(host_type)); + phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE)); object_property_add_child(OBJECT(machine), "i440fx", phb); object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM, OBJECT(ram_memory), &error_fatal); @@ -451,7 +450,7 @@ static void pc_compat_2_0_fn(MachineState *machine) #ifdef CONFIG_ISAPC static void pc_init_isa(MachineState *machine) { - pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE); + pc_init1(machine, TYPE_I440FX_PCI_DEVICE); } #endif @@ -461,9 +460,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine) const char *pci_type = xen_igd_gfx_pt_enabled() ? TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE; - pc_init1(machine, - TYPE_I440FX_PCI_HOST_BRIDGE, - pci_type); + pc_init1(machine, pci_type); } static void pc_xen_hvm_init(MachineState *machine) @@ -488,8 +485,7 @@ static void pc_xen_hvm_init(MachineState *machine) if (compat) { \ compat(machine); \ } \ - pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \ - TYPE_I440FX_PCI_DEVICE); \ + pc_init1(machine, TYPE_I440FX_PCI_DEVICE); \ } \ DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn) From ecca5ca549c13202504aacaf2885ccbed4c021e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 28 Feb 2024 09:17:40 +0100 Subject: [PATCH 22/43] hw/i386/pc: Have pc_init_isa() pass a NULL pci_type argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "isapc" machine only provides an ISA bus, not a PCI one, and doesn't instanciate any i440FX south bridge. Its machine class defines PCMachineClass::pci_enabled = false, and pc_init1() only uses the pci_type argument when pci_enabled is true. Since for this machine the argument is not used, passing NULL makes more sense. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Bernhard Beschow Message-Id: <20240301185936.95175-5-philmd@linaro.org> --- hw/i386/pc_piix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index e14dce951d..319bc4b180 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -450,7 +450,7 @@ static void pc_compat_2_0_fn(MachineState *machine) #ifdef CONFIG_ISAPC static void pc_init_isa(MachineState *machine) { - pc_init1(machine, TYPE_I440FX_PCI_DEVICE); + pc_init1(machine, NULL); } #endif From 0fad90955e3a56bfc45e623c1d96ae4a802ceda8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Mar 2024 23:41:33 +0100 Subject: [PATCH 23/43] hw/intc/apic: fix memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deliver_bitmask is allocated on the heap in apic_deliver(), but there are many paths in the function that return before the corresponding g_free() is reached. Fix this by switching to g_autofree and, while at it, also switch to g_new. Do the same in apic_deliver_irq() as well for consistency. Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14) Signed-off-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Bui Quang Minh Reviewed-by: Alex Bennée Message-ID: <20240304224133.267640-1-pbonzini@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/intc/apic.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 1d887d66b8..4186c57b34 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -291,14 +291,13 @@ static void apic_deliver_irq(uint32_t dest, uint8_t dest_mode, uint8_t delivery_mode, uint8_t vector_num, uint8_t trigger_mode) { - uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t)); + g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words); trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num, trigger_mode); apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode); apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode); - g_free(deliver_bitmask); } bool is_x2apic_mode(DeviceState *dev) @@ -662,7 +661,7 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode, APICCommonState *s = APIC(dev); APICCommonState *apic_iter; uint32_t deliver_bitmask_size = max_apic_words * sizeof(uint32_t); - uint32_t *deliver_bitmask = g_malloc(deliver_bitmask_size); + g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words); uint32_t current_apic_id; if (is_x2apic_mode(dev)) { @@ -708,7 +707,6 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode, } apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode); - g_free(deliver_bitmask); } static bool apic_check_pic(APICCommonState *s) From 965bc083103e1359679932feb6aadd35359c24ae Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Tue, 27 Feb 2024 17:55:48 +0100 Subject: [PATCH 24/43] qdev: Add a granule_mode property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a new enum type property allowing to set an IOMMU granule. Values are 4k, 8k, 16k, 64k and host. This latter indicates the vIOMMU granule will match the host page size. A subsequent patch will add such a property to the virtio-iommu device. Signed-off-by: Eric Auger Reviewed-by: Zhenzhong Duan Signed-off-by: Philippe Mathieu-Daudé Message-ID: <20240227165730.14099-2-eric.auger@redhat.com> --- hw/core/qdev-properties-system.c | 14 ++++++++++++++ include/hw/qdev-properties-system.h | 3 +++ qapi/virtio.json | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 1a396521d5..b45e90edb2 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -679,6 +679,20 @@ const PropertyInfo qdev_prop_mig_mode = { .set_default_value = qdev_propinfo_set_default_value_enum, }; +/* --- GranuleMode --- */ + +QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int)); + +const PropertyInfo qdev_prop_granule_mode = { + .name = "GranuleMode", + .description = "granule_mode values, " + "4k, 8k, 16k, 64k, host", + .enum_table = &GranuleMode_lookup, + .get = qdev_propinfo_get_enum, + .set = qdev_propinfo_set_enum, + .set_default_value = qdev_propinfo_set_default_value_enum, +}; + /* --- Reserved Region --- */ /* diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h index 06c359c190..626be87dd3 100644 --- a/include/hw/qdev-properties-system.h +++ b/include/hw/qdev-properties-system.h @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; extern const PropertyInfo qdev_prop_reserved_region; extern const PropertyInfo qdev_prop_multifd_compression; extern const PropertyInfo qdev_prop_mig_mode; +extern const PropertyInfo qdev_prop_granule_mode; extern const PropertyInfo qdev_prop_losttickpolicy; extern const PropertyInfo qdev_prop_blockdev_on_error; extern const PropertyInfo qdev_prop_bios_chs_trans; @@ -47,6 +48,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ MigMode) +#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \ + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode, GranuleMode) #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ LostTickPolicy) diff --git a/qapi/virtio.json b/qapi/virtio.json index a79013fe89..95745fdfd7 100644 --- a/qapi/virtio.json +++ b/qapi/virtio.json @@ -957,3 +957,21 @@ { 'struct': 'DummyVirtioForceArrays', 'data': { 'unused-iothread-vq-mapping': ['IOThreadVirtQueueMapping'] } } + +## +# @GranuleMode: +# +# @4k: granule page size of 4KiB +# +# @8k: granule page size of 8KiB +# +# @16k: granule page size of 16KiB +# +# @64k: granule page size of 64KiB +# +# @host: granule matches the host page size +# +# Since: 9.0 +## +{ 'enum': 'GranuleMode', + 'data': [ '4k', '8k', '16k', '64k', 'host' ] } From 46e23b2e424f60e5efd404901b4bcd1410bb8091 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Fri, 1 Mar 2024 19:01:10 +0100 Subject: [PATCH 25/43] hmp: Add option to info qtree to omit details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The output of info qtree monitor command is very long. Add an option to print a brief overview omitting all the details. Signed-off-by: BALATON Zoltan Reviewed-by: Dr. David Alan Gilbert Message-ID: <20240307183812.0105D4E6004@zero.eik.bme.hu> Signed-off-by: Philippe Mathieu-Daudé --- hmp-commands-info.hx | 6 +++--- system/qdev-monitor.c | 27 +++++++++++++++------------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index da120f82a3..ad1b1306e3 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -540,9 +540,9 @@ ERST { .name = "qtree", - .args_type = "", - .params = "", - .help = "show device tree", + .args_type = "brief:-b", + .params = "[-b]", + .help = "show device tree (-b: brief, omit properties)", .cmd = hmp_info_qtree, }, diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index a13db763e5..ad91e74181 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -744,7 +744,6 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) } #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__) -static void qbus_print(Monitor *mon, BusState *bus, int indent); static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props, int indent) @@ -784,13 +783,9 @@ static void bus_print_dev(BusState *bus, Monitor *mon, DeviceState *dev, int ind static void qdev_print(Monitor *mon, DeviceState *dev, int indent) { ObjectClass *class; - BusState *child; NamedGPIOList *ngl; NamedClockList *ncl; - qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)), - dev->id ? dev->id : ""); - indent += 2; QLIST_FOREACH(ngl, &dev->gpios, node) { if (ngl->num_in) { qdev_printf("gpio-in \"%s\" %d\n", ngl->name ? ngl->name : "", @@ -814,12 +809,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent) class = object_class_get_parent(class); } while (class != object_class_by_name(TYPE_DEVICE)); bus_print_dev(dev->parent_bus, mon, dev, indent); - QLIST_FOREACH(child, &dev->child_bus, sibling) { - qbus_print(mon, child, indent); - } } -static void qbus_print(Monitor *mon, BusState *bus, int indent) +static void qbus_print(Monitor *mon, BusState *bus, int indent, bool details) { BusChild *kid; @@ -827,16 +819,27 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent) indent += 2; qdev_printf("type %s\n", object_get_typename(OBJECT(bus))); QTAILQ_FOREACH(kid, &bus->children, sibling) { + BusState *child_bus; DeviceState *dev = kid->child; - qdev_print(mon, dev, indent); + qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)), + dev->id ? dev->id : ""); + if (details) { + qdev_print(mon, dev, indent + 2); + } + QLIST_FOREACH(child_bus, &dev->child_bus, sibling) { + qbus_print(mon, child_bus, indent + 2, details); + } } } #undef qdev_printf void hmp_info_qtree(Monitor *mon, const QDict *qdict) { - if (sysbus_get_default()) - qbus_print(mon, sysbus_get_default(), 0); + bool details = !qdict_get_try_bool(qdict, "brief", false); + + if (sysbus_get_default()) { + qbus_print(mon, sysbus_get_default(), 0, details); + } } void hmp_info_qdm(Monitor *mon, const QDict *qdict) From 78abf93cc7e2254401ccdb4df53e608927d81feb Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Mon, 4 Mar 2024 07:35:48 +0000 Subject: [PATCH 26/43] mac_newworld: change timebase frequency from 100MHz to 25MHz for mac99 machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MacOS X uses multiple techniques for calibrating timers depending upon the detected hardware. One of these calibration routines compares the change in the timebase against the KeyLargo timer and uses this to recalculate the clock frequency, timebase frequency and bus frequency if the calibration exceeds certain limits. This recalibration occurs despite the correct values being passed via the device tree, and is likely due to buggy firmware on some hardware. The timebase frequency of 100MHz was set way back in 2005 by commit fa296b0fb4 ("PIC fix - changed back TB frequency to 100 MHz") and with this value on a mac99,via=pmu machine the OSX 10.2 timer calibration incorrectly calculates the bus frequency as 400MHz instead of 100MHz. The most noticeable side-effect is the UI appears sluggish and not very responsive for normal use. Change the timebase frequency from 100MHz to 25MHz which matches that of a real G4 AGP machine (the closest match to QEMU's mac99 machine) and allows OSX 10.2 to correctly detect all of the clock frequency, timebase frequency and bus frequency. Tested on various MacOS images from OS 9.2 through to OSX 10.4, along with Linux and NetBSD and I was unable to find any regressions from this change. Signed-off-by: Mark Cave-Ayland Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240304073548.2098806-1-mark.cave-ayland@ilande.co.uk> Signed-off-by: Philippe Mathieu-Daudé --- hw/ppc/mac_newworld.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 3e796d2f6d..ff9e490c4e 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -77,7 +77,7 @@ #define MAX_IDE_BUS 2 #define CFG_ADDR 0xf0000510 -#define TBFREQ (100UL * 1000UL * 1000UL) +#define TBFREQ (25UL * 1000UL * 1000UL) #define CLOCKFREQ (900UL * 1000UL * 1000UL) #define BUSFREQ (100UL * 1000UL * 1000UL) From c9ee67c3c64cb161a092d9af6be0c17643d92be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= Date: Fri, 8 Mar 2024 16:27:19 +0100 Subject: [PATCH 27/43] hw/intc/grlib_irqmp: abort realize when ncpus value is out of range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even if the error is set, the build is not aborted when the ncpus value is wrong, the return is missing. Signed-off-by: Clément Chigot Reviewed-by: Peter Maydell Fixes: 6bf1478543 ("hw/intc/grlib_irqmp: add ncpus property") Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240308152719.591232-1-chigot@adacore.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/intc/grlib_irqmp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c index 144b121d48..c6c51a349c 100644 --- a/hw/intc/grlib_irqmp.c +++ b/hw/intc/grlib_irqmp.c @@ -356,6 +356,7 @@ static void grlib_irqmp_realize(DeviceState *dev, Error **errp) error_setg(errp, "Invalid ncpus properties: " "%u, must be 0 < ncpus =< %u.", irqmp->ncpus, IRQMP_MAX_CPU); + return; } qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS); From 3d6753ef183f54203973711ae60e893a525018b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Thu, 7 Mar 2024 13:05:34 +0100 Subject: [PATCH 28/43] docs/interop/firmware.json: Align examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adjust indentation for commit d23055b8db8 (qapi: Require descriptions and tagged sections to be indented). Signed-off-by: Thomas Weißschuh Reviewed-by: Markus Armbruster Message-ID: <20240307-qapi-firmware-json-v2-1-3b29eabb9b9a@linutronix.de> [PMD: Reword description using Markus suggestion] Signed-off-by: Philippe Mathieu-Daudé --- docs/interop/firmware.json | 374 ++++++++++++++++++------------------- 1 file changed, 187 insertions(+), 187 deletions(-) diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json index cc8f869186..a024f1b9bf 100644 --- a/docs/interop/firmware.json +++ b/docs/interop/firmware.json @@ -435,203 +435,203 @@ # # Examples: # -# { -# "description": "SeaBIOS", -# "interface-types": [ -# "bios" -# ], -# "mapping": { -# "device": "memory", -# "filename": "/usr/share/seabios/bios-256k.bin" -# }, -# "targets": [ -# { -# "architecture": "i386", -# "machines": [ -# "pc-i440fx-*", -# "pc-q35-*" -# ] +# { +# "description": "SeaBIOS", +# "interface-types": [ +# "bios" +# ], +# "mapping": { +# "device": "memory", +# "filename": "/usr/share/seabios/bios-256k.bin" # }, -# { -# "architecture": "x86_64", -# "machines": [ -# "pc-i440fx-*", -# "pc-q35-*" -# ] -# } -# ], -# "features": [ -# "acpi-s3", -# "acpi-s4" -# ], -# "tags": [ -# "CONFIG_BOOTSPLASH=n", -# "CONFIG_ROM_SIZE=256", -# "CONFIG_USE_SMM=n" -# ] -# } +# "targets": [ +# { +# "architecture": "i386", +# "machines": [ +# "pc-i440fx-*", +# "pc-q35-*" +# ] +# }, +# { +# "architecture": "x86_64", +# "machines": [ +# "pc-i440fx-*", +# "pc-q35-*" +# ] +# } +# ], +# "features": [ +# "acpi-s3", +# "acpi-s4" +# ], +# "tags": [ +# "CONFIG_BOOTSPLASH=n", +# "CONFIG_ROM_SIZE=256", +# "CONFIG_USE_SMM=n" +# ] +# } # -# { -# "description": "OVMF with SB+SMM, empty varstore", -# "interface-types": [ -# "uefi" -# ], -# "mapping": { -# "device": "flash", -# "executable": { -# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd", -# "format": "raw" +# { +# "description": "OVMF with SB+SMM, empty varstore", +# "interface-types": [ +# "uefi" +# ], +# "mapping": { +# "device": "flash", +# "executable": { +# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd", +# "format": "raw" +# }, +# "nvram-template": { +# "filename": "/usr/share/OVMF/OVMF_VARS.fd", +# "format": "raw" +# } # }, -# "nvram-template": { -# "filename": "/usr/share/OVMF/OVMF_VARS.fd", -# "format": "raw" -# } -# }, -# "targets": [ -# { -# "architecture": "x86_64", -# "machines": [ -# "pc-q35-*" -# ] -# } -# ], -# "features": [ -# "acpi-s3", -# "amd-sev", -# "requires-smm", -# "secure-boot", -# "verbose-dynamic" -# ], -# "tags": [ -# "-a IA32", -# "-a X64", -# "-p OvmfPkg/OvmfPkgIa32X64.dsc", -# "-t GCC48", -# "-b DEBUG", -# "-D SMM_REQUIRE", -# "-D SECURE_BOOT_ENABLE", -# "-D FD_SIZE_4MB" -# ] -# } +# "targets": [ +# { +# "architecture": "x86_64", +# "machines": [ +# "pc-q35-*" +# ] +# } +# ], +# "features": [ +# "acpi-s3", +# "amd-sev", +# "requires-smm", +# "secure-boot", +# "verbose-dynamic" +# ], +# "tags": [ +# "-a IA32", +# "-a X64", +# "-p OvmfPkg/OvmfPkgIa32X64.dsc", +# "-t GCC48", +# "-b DEBUG", +# "-D SMM_REQUIRE", +# "-D SECURE_BOOT_ENABLE", +# "-D FD_SIZE_4MB" +# ] +# } # -# { -# "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled", -# "interface-types": [ -# "uefi" -# ], -# "mapping": { -# "device": "flash", -# "executable": { -# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd", -# "format": "raw" +# { +# "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled", +# "interface-types": [ +# "uefi" +# ], +# "mapping": { +# "device": "flash", +# "executable": { +# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd", +# "format": "raw" +# }, +# "nvram-template": { +# "filename": "/usr/share/OVMF/OVMF_VARS.secboot.fd", +# "format": "raw" +# } # }, -# "nvram-template": { -# "filename": "/usr/share/OVMF/OVMF_VARS.secboot.fd", -# "format": "raw" -# } -# }, -# "targets": [ -# { -# "architecture": "x86_64", -# "machines": [ -# "pc-q35-*" -# ] -# } -# ], -# "features": [ -# "acpi-s3", -# "amd-sev", -# "enrolled-keys", -# "requires-smm", -# "secure-boot", -# "verbose-dynamic" -# ], -# "tags": [ -# "-a IA32", -# "-a X64", -# "-p OvmfPkg/OvmfPkgIa32X64.dsc", -# "-t GCC48", -# "-b DEBUG", -# "-D SMM_REQUIRE", -# "-D SECURE_BOOT_ENABLE", -# "-D FD_SIZE_4MB" -# ] -# } +# "targets": [ +# { +# "architecture": "x86_64", +# "machines": [ +# "pc-q35-*" +# ] +# } +# ], +# "features": [ +# "acpi-s3", +# "amd-sev", +# "enrolled-keys", +# "requires-smm", +# "secure-boot", +# "verbose-dynamic" +# ], +# "tags": [ +# "-a IA32", +# "-a X64", +# "-p OvmfPkg/OvmfPkgIa32X64.dsc", +# "-t GCC48", +# "-b DEBUG", +# "-D SMM_REQUIRE", +# "-D SECURE_BOOT_ENABLE", +# "-D FD_SIZE_4MB" +# ] +# } # -# { -# "description": "OVMF with SEV-ES support", -# "interface-types": [ -# "uefi" -# ], -# "mapping": { -# "device": "flash", -# "executable": { -# "filename": "/usr/share/OVMF/OVMF_CODE.fd", -# "format": "raw" +# { +# "description": "OVMF with SEV-ES support", +# "interface-types": [ +# "uefi" +# ], +# "mapping": { +# "device": "flash", +# "executable": { +# "filename": "/usr/share/OVMF/OVMF_CODE.fd", +# "format": "raw" +# }, +# "nvram-template": { +# "filename": "/usr/share/OVMF/OVMF_VARS.fd", +# "format": "raw" +# } # }, -# "nvram-template": { -# "filename": "/usr/share/OVMF/OVMF_VARS.fd", -# "format": "raw" -# } -# }, -# "targets": [ -# { -# "architecture": "x86_64", -# "machines": [ -# "pc-q35-*" -# ] -# } -# ], -# "features": [ -# "acpi-s3", -# "amd-sev", -# "amd-sev-es", -# "verbose-dynamic" -# ], -# "tags": [ -# "-a X64", -# "-p OvmfPkg/OvmfPkgX64.dsc", -# "-t GCC48", -# "-b DEBUG", -# "-D FD_SIZE_4MB" -# ] -# } +# "targets": [ +# { +# "architecture": "x86_64", +# "machines": [ +# "pc-q35-*" +# ] +# } +# ], +# "features": [ +# "acpi-s3", +# "amd-sev", +# "amd-sev-es", +# "verbose-dynamic" +# ], +# "tags": [ +# "-a X64", +# "-p OvmfPkg/OvmfPkgX64.dsc", +# "-t GCC48", +# "-b DEBUG", +# "-D FD_SIZE_4MB" +# ] +# } # -# { -# "description": "UEFI firmware for ARM64 virtual machines", -# "interface-types": [ -# "uefi" -# ], -# "mapping": { -# "device": "flash", -# "executable": { -# "filename": "/usr/share/AAVMF/AAVMF_CODE.fd", -# "format": "raw" +# { +# "description": "UEFI firmware for ARM64 virtual machines", +# "interface-types": [ +# "uefi" +# ], +# "mapping": { +# "device": "flash", +# "executable": { +# "filename": "/usr/share/AAVMF/AAVMF_CODE.fd", +# "format": "raw" +# }, +# "nvram-template": { +# "filename": "/usr/share/AAVMF/AAVMF_VARS.fd", +# "format": "raw" +# } # }, -# "nvram-template": { -# "filename": "/usr/share/AAVMF/AAVMF_VARS.fd", -# "format": "raw" -# } -# }, -# "targets": [ -# { -# "architecture": "aarch64", -# "machines": [ -# "virt-*" -# ] -# } -# ], -# "features": [ +# "targets": [ +# { +# "architecture": "aarch64", +# "machines": [ +# "virt-*" +# ] +# } +# ], +# "features": [ # -# ], -# "tags": [ -# "-a AARCH64", -# "-p ArmVirtPkg/ArmVirtQemu.dsc", -# "-t GCC48", -# "-b DEBUG", -# "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000" -# ] -# } +# ], +# "tags": [ +# "-a AARCH64", +# "-p ArmVirtPkg/ArmVirtQemu.dsc", +# "-t GCC48", +# "-b DEBUG", +# "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000" +# ] +# } ## { 'struct' : 'Firmware', 'data' : { 'description' : 'str', From 56fa4f346a32b41d2fccd24c86cbf3390fc510e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Thu, 7 Mar 2024 13:05:35 +0100 Subject: [PATCH 29/43] docs/interop/firmware.json: Fix doc for FirmwareFlashMode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc title did not match the actual definition. Fixes: 2720ceda05 ("docs: expand firmware descriptor to allow flash without NVRAM") Signed-off-by: Thomas Weißschuh Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240307-qapi-firmware-json-v2-2-3b29eabb9b9a@linutronix.de> Signed-off-by: Philippe Mathieu-Daudé --- docs/interop/firmware.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json index a024f1b9bf..54a1fc6c10 100644 --- a/docs/interop/firmware.json +++ b/docs/interop/firmware.json @@ -223,7 +223,7 @@ ## -# @FirmwareFlashType: +# @FirmwareFlashMode: # # Describes how the firmware build handles code versus variable # persistence. From 72d346f3b8504804df047166164af52af4f95708 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:36 +0800 Subject: [PATCH 30/43] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "parameter=0" SMP configurations have been marked as deprecated since v6.2. For these cases, -smp currently returns the warning and adjusts the zeroed parameters to 1 by default. Remove the above compatibility logic in v9.0, and return error directly if any -smp parameter is set as 0. Signed-off-by: Zhao Liu Reviewed-by: Thomas Huth Reviewed-by: Prasad Pandit Message-ID: <20240308160148.3130837-2-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- docs/about/deprecated.rst | 16 ---------------- docs/about/removed-features.rst | 15 +++++++++++++++ hw/core/machine-smp.c | 5 +++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 8565644da6..6e2f557682 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -36,22 +36,6 @@ and will cause a warning. The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on`` rather than ``delay=off``. -``-smp`` ("parameter=0" SMP configurations) (since 6.2) -''''''''''''''''''''''''''''''''''''''''''''''''''''''' - -Specified CPU topology parameters must be greater than zero. - -In the SMP configuration, users should either provide a CPU topology -parameter with a reasonable value (greater than zero) or just omit it -and QEMU will compute the missing value. - -However, historically it was implicitly allowed for users to provide -a parameter with zero value, which is meaningless and could also possibly -cause unexpected results in the -smp parsing. So support for this kind of -configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will -be removed in the near future, users have to ensure that all the topology -members described with -smp are greater than zero. - Plugin argument passing through ``arg=`` (since 6.1) '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 417a0e4fa1..f9cf874f7b 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -489,6 +489,21 @@ The ``-singlestep`` option has been turned into an accelerator property, and given a name that better reflects what it actually does. Use ``-accel tcg,one-insn-per-tb=on`` instead. +``-smp`` ("parameter=0" SMP configurations) (removed in 9.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Specified CPU topology parameters must be greater than zero. + +In the SMP configuration, users should either provide a CPU topology +parameter with a reasonable value (greater than zero) or just omit it +and QEMU will compute the missing value. + +However, historically it was implicitly allowed for users to provide +a parameter with zero value, which is meaningless and could also possibly +cause unexpected results in the -smp parsing. So support for this kind of +configurations (e.g. -smp 8,sockets=0) is removed since 9.0, users have +to ensure that all the topology members described with -smp are greater +than zero. User-mode emulator command line arguments ----------------------------------------- diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 25019c91ee..96533886b1 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -105,8 +105,9 @@ void machine_parse_smp_config(MachineState *ms, (config->has_cores && config->cores == 0) || (config->has_threads && config->threads == 0) || (config->has_maxcpus && config->maxcpus == 0)) { - warn_report("Deprecated CPU topology (considered invalid): " - "CPU topology parameters must be greater than zero"); + error_setg(errp, "Invalid CPU topology: " + "CPU topology parameters must be greater than zero"); + return; } /* From 54c4ea8f3ae614054079395842128a856a73dbf9 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:37 +0800 Subject: [PATCH 31/43] hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, it was allowed for users to specify the unsupported topology parameter as "1". For example, x86 PC machine doesn't support drawer/book/cluster topology levels, but user could specify "-smp drawers=1,books=1,clusters=1". This is meaningless and confusing, so that the support for this kind of configurations is marked deprecated since 9.0. And report warning message for such case like: qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid): Unsupported clusters parameter mustn't be specified as 1 qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid): Unsupported books parameter mustn't be specified as 1 qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid): Unsupported drawers parameter mustn't be specified as 1 Users have to ensure that all the topology members described with -smp are supported by the target machine. Signed-off-by: Zhao Liu Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-3-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- docs/about/deprecated.rst | 14 +++++++++ hw/core/machine-smp.c | 65 +++++++++++++++++++++++++++++---------- 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 6e2f557682..dfd681cd02 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -57,6 +57,20 @@ The ``-p`` option pretends to control the host page size. However, it is not possible to change the host page size, and using the option only causes failures. +``-smp`` (Unsupported "parameter=1" SMP configurations) (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Specified CPU topology parameters must be supported by the machine. + +In the SMP configuration, users should provide the CPU topology parameters that +are supported by the target machine. + +However, historically it was allowed for users to specify the unsupported +topology parameter as "1", which is meaningless. So support for this kind of +configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is +marked deprecated since 9.0, users have to ensure that all the topology members +described with -smp are supported by the target machine. + QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 96533886b1..50a5a40dbc 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -112,30 +112,61 @@ void machine_parse_smp_config(MachineState *ms, /* * If not supported by the machine, a topology parameter must be - * omitted or specified equal to 1. + * omitted. */ - if (!mc->smp_props.dies_supported && dies > 1) { - error_setg(errp, "dies not supported by this machine's CPU topology"); - return; + if (!mc->smp_props.clusters_supported && config->has_clusters) { + if (config->clusters > 1) { + error_setg(errp, "clusters not supported by this " + "machine's CPU topology"); + return; + } else { + /* Here clusters only equals 1 since we've checked zero case. */ + warn_report("Deprecated CPU topology (considered invalid): " + "Unsupported clusters parameter mustn't be " + "specified as 1"); + } } - if (!mc->smp_props.clusters_supported && clusters > 1) { - error_setg(errp, "clusters not supported by this machine's CPU topology"); - return; - } - - dies = dies > 0 ? dies : 1; clusters = clusters > 0 ? clusters : 1; - if (!mc->smp_props.books_supported && books > 1) { - error_setg(errp, "books not supported by this machine's CPU topology"); - return; + if (!mc->smp_props.dies_supported && config->has_dies) { + if (config->dies > 1) { + error_setg(errp, "dies not supported by this " + "machine's CPU topology"); + return; + } else { + /* Here dies only equals 1 since we've checked zero case. */ + warn_report("Deprecated CPU topology (considered invalid): " + "Unsupported dies parameter mustn't be " + "specified as 1"); + } + } + dies = dies > 0 ? dies : 1; + + if (!mc->smp_props.books_supported && config->has_books) { + if (config->books > 1) { + error_setg(errp, "books not supported by this " + "machine's CPU topology"); + return; + } else { + /* Here books only equals 1 since we've checked zero case. */ + warn_report("Deprecated CPU topology (considered invalid): " + "Unsupported books parameter mustn't be " + "specified as 1"); + } } books = books > 0 ? books : 1; - if (!mc->smp_props.drawers_supported && drawers > 1) { - error_setg(errp, - "drawers not supported by this machine's CPU topology"); - return; + if (!mc->smp_props.drawers_supported && config->has_drawers) { + if (config->drawers > 1) { + error_setg(errp, "drawers not supported by this " + "machine's CPU topology"); + return; + } else { + /* Here drawers only equals 1 since we've checked zero case. */ + warn_report("Deprecated CPU topology (considered invalid): " + "Unsupported drawers parameter mustn't be " + "specified as 1"); + } } drawers = drawers > 0 ? drawers : 1; From 4503dcf77b9006b56dba01a407bffbb9a37ea38e Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:38 +0800 Subject: [PATCH 32/43] hw/core/machine-smp: Calculate total CPUs once in machine_parse_smp_config() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In machine_parse_smp_config(), the number of total CPUs is calculated by: drawers * books * sockets * dies * clusters * cores * threads To avoid missing the future new topology level, use a local variable to cache the calculation result so that total CPUs are only calculated once. Signed-off-by: Zhao Liu Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240308160148.3130837-4-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine-smp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 50a5a40dbc..27864c9507 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -91,6 +91,7 @@ void machine_parse_smp_config(MachineState *ms, unsigned cores = config->has_cores ? config->cores : 0; unsigned threads = config->has_threads ? config->threads : 0; unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; + unsigned total_cpus; /* * Specified CPU topology parameters must be greater than zero, @@ -211,8 +212,8 @@ void machine_parse_smp_config(MachineState *ms, } } - maxcpus = maxcpus > 0 ? maxcpus : drawers * books * sockets * dies * - clusters * cores * threads; + total_cpus = drawers * books * sockets * dies * clusters * cores * threads; + maxcpus = maxcpus > 0 ? maxcpus : total_cpus; cpus = cpus > 0 ? cpus : maxcpus; ms->smp.cpus = cpus; @@ -228,8 +229,7 @@ void machine_parse_smp_config(MachineState *ms, mc->smp_props.has_clusters = config->has_clusters; /* sanity-check of the computed topology */ - if (drawers * books * sockets * dies * clusters * cores * threads != - maxcpus) { + if (total_cpus != maxcpus) { g_autofree char *topo_msg = cpu_hierarchy_to_string(ms); error_setg(errp, "Invalid CPU topology: " "product of the hierarchy must match maxcpus: " From a4f9386071c4a04fe581ac3750e2d4d61c683a6c Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:39 +0800 Subject: [PATCH 33/43] tests/unit/test-smp-parse: Drop the unsupported "dies=1" case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unsupported "parameter=1" SMP configurations is marked as deprecated, so drop the related test case. Signed-off-by: Zhao Liu Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-5-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 24972666a7..1874bea086 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -607,11 +607,6 @@ static void test_generic_valid(const void *opaque) unsupported_params_init(mc, &data); smp_parse_test(ms, &data, true); - - /* Unsupported parameters can be provided with their values as 1 */ - data.config.has_dies = true; - data.config.dies = 1; - smp_parse_test(ms, &data, true); } object_unref(obj); From 803f9714bfc12cf19488c61a0db4b1edf696e7d4 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:40 +0800 Subject: [PATCH 34/43] tests/unit/test-smp-parse: Use CPU number macros in invalid topology case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use MAX_CPUS/MIN_CPUS macros in invalid topology case. This gives us the flexibility to change the maximum and minimum CPU limits. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-6-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 1874bea086..84e3422774 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -323,15 +323,21 @@ static const struct SMPTestData data_generic_invalid[] = { "sockets (2) * cores (4) * threads (2) " "== maxcpus (16) < smp_cpus (18)", }, { - /* config: -smp 1 - * should tweak the supported min CPUs to 2 for testing */ - .config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0), + /* + * config: -smp 1 + * The test machine should tweak the supported min CPUs to + * 2 (MIN_CPUS + 1) for testing. + */ + .config = SMP_CONFIG_GENERIC(T, MIN_CPUS, F, 0, F, 0, F, 0, F, 0), .expect_error = "Invalid SMP CPUs 1. The min CPUs supported " "by machine '" SMP_MACHINE_NAME "' is 2", }, { - /* config: -smp 512 - * should tweak the supported max CPUs to 511 for testing */ - .config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0), + /* + * config: -smp 512 + * The test machine should tweak the supported max CPUs to + * 511 (MAX_CPUS - 1) for testing. + */ + .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0), .expect_error = "Invalid SMP CPUs 512. The max CPUs supported " "by machine '" SMP_MACHINE_NAME "' is 511", }, @@ -575,8 +581,8 @@ static void machine_generic_invalid_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); /* Force invalid min CPUs and max CPUs */ - mc->min_cpus = 2; - mc->max_cpus = 511; + mc->min_cpus = MIN_CPUS + 1; + mc->max_cpus = MAX_CPUS - 1; } static void machine_with_dies_class_init(ObjectClass *oc, void *data) From f4e65d64bdcee098d660fd427dd41ef557985d68 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:41 +0800 Subject: [PATCH 35/43] tests/unit/test-smp-parse: Bump max_cpus to 4096 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The q35 machine is trying to support up to 4096 vCPUs [1], so it's necessary to bump max_cpus in test-smp-parse to 4096 to cover the topological needs of future machines. [1]: https://lore.kernel.org/qemu-devel/20240228143351.3967-1-anisinha@redhat.com/ Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-7-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 84e3422774..2eb9533bc5 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -20,8 +20,8 @@ #define T true #define F false -#define MIN_CPUS 1 /* set the min CPUs supported by the machine as 1 */ -#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */ +#define MIN_CPUS 1 /* set the min CPUs supported by the machine as 1 */ +#define MAX_CPUS 4096 /* set the max CPUs supported by the machine as 4096 */ #define SMP_MACHINE_NAME "TEST-SMP" @@ -333,13 +333,13 @@ static const struct SMPTestData data_generic_invalid[] = { "by machine '" SMP_MACHINE_NAME "' is 2", }, { /* - * config: -smp 512 + * config: -smp 4096 * The test machine should tweak the supported max CPUs to - * 511 (MAX_CPUS - 1) for testing. + * 4095 (MAX_CPUS - 1) for testing. */ - .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0), - .expect_error = "Invalid SMP CPUs 512. The max CPUs supported " - "by machine '" SMP_MACHINE_NAME "' is 511", + .config = SMP_CONFIG_GENERIC(T, 4096, F, 0, F, 0, F, 0, F, 0), + .expect_error = "Invalid SMP CPUs 4096. The max CPUs supported " + "by machine '" SMP_MACHINE_NAME "' is 4095", }, }; From dc583442dc877223d0ed09b1e33caf3d2bebe589 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:42 +0800 Subject: [PATCH 36/43] tests/unit/test-smp-parse: Make test cases aware of the book/drawer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, -smp supports 2 more new levels: book and drawer. It is necessary to consider the effects of book and drawer in the test cases to ensure that the calculations are correct. This is also the preparation to add new book and drawer test cases. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-8-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 2eb9533bc5..f656bbb6da 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -384,6 +384,8 @@ static char *smp_config_to_string(const SMPConfiguration *config) return g_strdup_printf( "(SMPConfiguration) {\n" " .has_cpus = %5s, cpus = %" PRId64 ",\n" + " .has_drawers = %5s, drawers = %" PRId64 ",\n" + " .has_books = %5s, books = %" PRId64 ",\n" " .has_sockets = %5s, sockets = %" PRId64 ",\n" " .has_dies = %5s, dies = %" PRId64 ",\n" " .has_clusters = %5s, clusters = %" PRId64 ",\n" @@ -392,6 +394,8 @@ static char *smp_config_to_string(const SMPConfiguration *config) " .has_maxcpus = %5s, maxcpus = %" PRId64 ",\n" "}", config->has_cpus ? "true" : "false", config->cpus, + config->has_drawers ? "true" : "false", config->drawers, + config->has_books ? "true" : "false", config->books, config->has_sockets ? "true" : "false", config->sockets, config->has_dies ? "true" : "false", config->dies, config->has_clusters ? "true" : "false", config->clusters, @@ -404,10 +408,10 @@ static char *smp_config_to_string(const SMPConfiguration *config) static unsigned int cpu_topology_get_threads_per_socket(const CpuTopology *topo) { /* Check the divisor to avoid invalid topology examples causing SIGFPE. */ - if (!topo->sockets) { + if (!topo->drawers || !topo->books || !topo->sockets) { return 0; } else { - return topo->max_cpus / topo->sockets; + return topo->max_cpus / topo->drawers / topo->books / topo->sockets; } } @@ -429,6 +433,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo, return g_strdup_printf( "(CpuTopology) {\n" " .cpus = %u,\n" + " .drawers = %u,\n" + " .books = %u,\n" " .sockets = %u,\n" " .dies = %u,\n" " .clusters = %u,\n" @@ -438,7 +444,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo, " .threads_per_socket = %u,\n" " .cores_per_socket = %u,\n" "}", - topo->cpus, topo->sockets, topo->dies, topo->clusters, + topo->cpus, topo->drawers, topo->books, + topo->sockets, topo->dies, topo->clusters, topo->cores, topo->threads, topo->max_cpus, threads_per_socket, cores_per_socket); } @@ -473,6 +480,8 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config, if (is_valid) { if ((err == NULL) && (ms->smp.cpus == expect_topo->cpus) && + (ms->smp.drawers == expect_topo->drawers) && + (ms->smp.books == expect_topo->books) && (ms->smp.sockets == expect_topo->sockets) && (ms->smp.dies == expect_topo->dies) && (ms->smp.clusters == expect_topo->clusters) && @@ -564,6 +573,16 @@ static void unsupported_params_init(const MachineClass *mc, SMPTestData *data) data->expect_prefer_sockets.clusters = 1; data->expect_prefer_cores.clusters = 1; } + + if (!mc->smp_props.books_supported) { + data->expect_prefer_sockets.books = 1; + data->expect_prefer_cores.books = 1; + } + + if (!mc->smp_props.drawers_supported) { + data->expect_prefer_sockets.drawers = 1; + data->expect_prefer_cores.drawers = 1; + } } static void machine_base_class_init(ObjectClass *oc, void *data) From f0fe1cd8b8e48abc3b7948dada49de9016323019 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:43 +0800 Subject: [PATCH 37/43] tests/unit/test-smp-parse: Test "books" parameter in -smp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although book was introduced to -smp along with drawer by s390 machine, as a general topology level in QEMU that may be reused by other arches in the future, it is desirable to cover this parameter's parsing in a separate case. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-9-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 105 ++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index f656bbb6da..090238ab23 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -75,6 +75,22 @@ .has_maxcpus = hf, .maxcpus = f, \ } +/* + * Currently a 5-level topology hierarchy is supported on s390 ccw machines + * -drawers/books/sockets/cores/threads + */ +#define SMP_CONFIG_WITH_BOOKS_DRAWERS(ha, a, hb, b, hc, c, hd, \ + d, he, e, hf, f, hg, g) \ + { \ + .has_cpus = ha, .cpus = a, \ + .has_drawers = hb, .drawers = b, \ + .has_books = hc, .books = c, \ + .has_sockets = hd, .sockets = d, \ + .has_cores = he, .cores = e, \ + .has_threads = hf, .threads = f, \ + .has_maxcpus = hg, .maxcpus = g, \ + } + /** * @config - the given SMP configuration * @expect_prefer_sockets - the expected parsing result for the @@ -308,6 +324,11 @@ static const struct SMPTestData data_generic_invalid[] = { /* config: -smp 2,clusters=2 */ .config = SMP_CONFIG_WITH_CLUSTERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0), .expect_error = "clusters not supported by this machine's CPU topology", + }, { + /* config: -smp 2,books=2 */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F, + 0, F, 0, F, 0, F, 0), + .expect_error = "books not supported by this machine's CPU topology", }, { /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8), @@ -379,6 +400,26 @@ static const struct SMPTestData data_with_clusters_invalid[] = { }, }; +static const struct SMPTestData data_with_books_invalid[] = { + { + /* config: -smp 16,books=2,sockets=2,cores=4,threads=2,maxcpus=16 */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, F, 1, T, 2, T, + 2, T, 4, T, 2, T, 16), + .expect_error = "Invalid CPU topology: " + "product of the hierarchy must match maxcpus: " + "books (2) * sockets (2) * cores (4) * threads (2) " + "!= maxcpus (16)", + }, { + /* config: -smp 34,books=2,sockets=2,cores=4,threads=2,maxcpus=32 */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, F, 1, T, 2, T, + 2, T, 4, T, 2, T, 32), + .expect_error = "Invalid CPU topology: " + "maxcpus must be equal to or greater than smp: " + "books (2) * sockets (2) * cores (4) * threads (2) " + "== maxcpus (32) < smp_cpus (34)", + }, +}; + static char *smp_config_to_string(const SMPConfiguration *config) { return g_strdup_printf( @@ -618,6 +659,13 @@ static void machine_with_clusters_class_init(ObjectClass *oc, void *data) mc->smp_props.clusters_supported = true; } +static void machine_with_books_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->smp_props.books_supported = true; +} + static void test_generic_valid(const void *opaque) { const char *machine_type = opaque; @@ -756,6 +804,56 @@ static void test_with_clusters(const void *opaque) object_unref(obj); } +static void test_with_books(const void *opaque) +{ + const char *machine_type = opaque; + Object *obj = object_new(machine_type); + MachineState *ms = MACHINE(obj); + MachineClass *mc = MACHINE_GET_CLASS(obj); + SMPTestData data = {}; + unsigned int num_books = 2; + int i; + + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { + data = data_generic_valid[i]; + unsupported_params_init(mc, &data); + + /* when books parameter is omitted, it will be set as 1 */ + data.expect_prefer_sockets.books = 1; + data.expect_prefer_cores.books = 1; + + smp_parse_test(ms, &data, true); + + /* when books parameter is specified */ + data.config.has_books = true; + data.config.books = num_books; + if (data.config.has_cpus) { + data.config.cpus *= num_books; + } + if (data.config.has_maxcpus) { + data.config.maxcpus *= num_books; + } + + data.expect_prefer_sockets.books = num_books; + data.expect_prefer_sockets.cpus *= num_books; + data.expect_prefer_sockets.max_cpus *= num_books; + data.expect_prefer_cores.books = num_books; + data.expect_prefer_cores.cpus *= num_books; + data.expect_prefer_cores.max_cpus *= num_books; + + smp_parse_test(ms, &data, true); + } + + for (i = 0; i < ARRAY_SIZE(data_with_books_invalid); i++) { + data = data_with_books_invalid[i]; + unsupported_params_init(mc, &data); + + smp_parse_test(ms, &data, false); + } + + object_unref(obj); +} + /* Type info of the tested machine */ static const TypeInfo smp_machine_types[] = { { @@ -780,6 +878,10 @@ static const TypeInfo smp_machine_types[] = { .name = MACHINE_TYPE_NAME("smp-with-clusters"), .parent = TYPE_MACHINE, .class_init = machine_with_clusters_class_init, + }, { + .name = MACHINE_TYPE_NAME("smp-with-books"), + .parent = TYPE_MACHINE, + .class_init = machine_with_books_class_init, } }; @@ -803,6 +905,9 @@ int main(int argc, char *argv[]) g_test_add_data_func("/test-smp-parse/with_clusters", MACHINE_TYPE_NAME("smp-with-clusters"), test_with_clusters); + g_test_add_data_func("/test-smp-parse/with_books", + MACHINE_TYPE_NAME("smp-with-books"), + test_with_books); g_test_run(); From 5a4c4148cb27b13f4517220622ddf526e484ed99 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:44 +0800 Subject: [PATCH 38/43] tests/unit/test-smp-parse: Test "drawers" parameter in -smp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although drawer was introduced to -smp along with book by s390 machine, as a general topology level in QEMU that may be reused by other arches in the future, it is desirable to cover this parameter's parsing in a separate case. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-10-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 89 +++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 090238ab23..aea1b2e73a 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -329,6 +329,11 @@ static const struct SMPTestData data_generic_invalid[] = { .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0, F, 0), .expect_error = "books not supported by this machine's CPU topology", + }, { + /* config: -smp 2,drawers=2 */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, T, 2, F, 0, F, + 0, F, 0, F, 0, F, 0), + .expect_error = "drawers not supported by this machine's CPU topology", }, { /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8), @@ -420,6 +425,26 @@ static const struct SMPTestData data_with_books_invalid[] = { }, }; +static const struct SMPTestData data_with_drawers_invalid[] = { + { + /* config: -smp 16,drawers=2,sockets=2,cores=4,threads=2,maxcpus=16 */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, T, 2, F, 1, T, + 2, T, 4, T, 2, T, 16), + .expect_error = "Invalid CPU topology: " + "product of the hierarchy must match maxcpus: " + "drawers (2) * sockets (2) * cores (4) * threads (2) " + "!= maxcpus (16)", + }, { + /* config: -smp 34,drawers=2,sockets=2,cores=4,threads=2,maxcpus=32 */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, T, 2, F, 1, T, + 2, T, 4, T, 2, T, 32), + .expect_error = "Invalid CPU topology: " + "maxcpus must be equal to or greater than smp: " + "drawers (2) * sockets (2) * cores (4) * threads (2) " + "== maxcpus (32) < smp_cpus (34)", + }, +}; + static char *smp_config_to_string(const SMPConfiguration *config) { return g_strdup_printf( @@ -666,6 +691,13 @@ static void machine_with_books_class_init(ObjectClass *oc, void *data) mc->smp_props.books_supported = true; } +static void machine_with_drawers_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->smp_props.drawers_supported = true; +} + static void test_generic_valid(const void *opaque) { const char *machine_type = opaque; @@ -854,6 +886,56 @@ static void test_with_books(const void *opaque) object_unref(obj); } +static void test_with_drawers(const void *opaque) +{ + const char *machine_type = opaque; + Object *obj = object_new(machine_type); + MachineState *ms = MACHINE(obj); + MachineClass *mc = MACHINE_GET_CLASS(obj); + SMPTestData data = {}; + unsigned int num_drawers = 2; + int i; + + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { + data = data_generic_valid[i]; + unsupported_params_init(mc, &data); + + /* when drawers parameter is omitted, it will be set as 1 */ + data.expect_prefer_sockets.drawers = 1; + data.expect_prefer_cores.drawers = 1; + + smp_parse_test(ms, &data, true); + + /* when drawers parameter is specified */ + data.config.has_drawers = true; + data.config.drawers = num_drawers; + if (data.config.has_cpus) { + data.config.cpus *= num_drawers; + } + if (data.config.has_maxcpus) { + data.config.maxcpus *= num_drawers; + } + + data.expect_prefer_sockets.drawers = num_drawers; + data.expect_prefer_sockets.cpus *= num_drawers; + data.expect_prefer_sockets.max_cpus *= num_drawers; + data.expect_prefer_cores.drawers = num_drawers; + data.expect_prefer_cores.cpus *= num_drawers; + data.expect_prefer_cores.max_cpus *= num_drawers; + + smp_parse_test(ms, &data, true); + } + + for (i = 0; i < ARRAY_SIZE(data_with_drawers_invalid); i++) { + data = data_with_drawers_invalid[i]; + unsupported_params_init(mc, &data); + + smp_parse_test(ms, &data, false); + } + + object_unref(obj); +} + /* Type info of the tested machine */ static const TypeInfo smp_machine_types[] = { { @@ -882,6 +964,10 @@ static const TypeInfo smp_machine_types[] = { .name = MACHINE_TYPE_NAME("smp-with-books"), .parent = TYPE_MACHINE, .class_init = machine_with_books_class_init, + }, { + .name = MACHINE_TYPE_NAME("smp-with-drawers"), + .parent = TYPE_MACHINE, + .class_init = machine_with_drawers_class_init, } }; @@ -908,6 +994,9 @@ int main(int argc, char *argv[]) g_test_add_data_func("/test-smp-parse/with_books", MACHINE_TYPE_NAME("smp-with-books"), test_with_books); + g_test_add_data_func("/test-smp-parse/with_drawers", + MACHINE_TYPE_NAME("smp-with-drawers"), + test_with_drawers); g_test_run(); From 3f90fb089eb0cb02bd40257c5176f2ad0874e02b Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:45 +0800 Subject: [PATCH 39/43] tests/unit/test-smp-parse: Test "drawers" and "books" combination case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since s390 machine supports both "drawers" and "books" in -smp, add the "drawers" and "books" combination test case to match the actual topology usage scenario. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-11-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 103 ++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index aea1b2e73a..0cf6115198 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -445,6 +445,33 @@ static const struct SMPTestData data_with_drawers_invalid[] = { }, }; +static const struct SMPTestData data_with_drawers_books_invalid[] = { + { + /* + * config: -smp 200,drawers=2,books=2,sockets=2,cores=4,\ + * threads=2,maxcpus=200 + */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 200, T, 3, T, 5, T, + 2, T, 4, T, 2, T, 200), + .expect_error = "Invalid CPU topology: " + "product of the hierarchy must match maxcpus: " + "drawers (3) * books (5) * sockets (2) * " + "cores (4) * threads (2) != maxcpus (200)", + }, { + /* + * config: -smp 242,drawers=2,books=2,sockets=2,cores=4,\ + * threads=2,maxcpus=240 + */ + .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 242, T, 3, T, 5, T, + 2, T, 4, T, 2, T, 240), + .expect_error = "Invalid CPU topology: " + "maxcpus must be equal to or greater than smp: " + "drawers (3) * books (5) * sockets (2) * " + "cores (4) * threads (2) " + "== maxcpus (240) < smp_cpus (242)", + }, +}; + static char *smp_config_to_string(const SMPConfiguration *config) { return g_strdup_printf( @@ -698,6 +725,14 @@ static void machine_with_drawers_class_init(ObjectClass *oc, void *data) mc->smp_props.drawers_supported = true; } +static void machine_with_drawers_books_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->smp_props.drawers_supported = true; + mc->smp_props.books_supported = true; +} + static void test_generic_valid(const void *opaque) { const char *machine_type = opaque; @@ -936,6 +971,67 @@ static void test_with_drawers(const void *opaque) object_unref(obj); } +static void test_with_drawers_books(const void *opaque) +{ + const char *machine_type = opaque; + Object *obj = object_new(machine_type); + MachineState *ms = MACHINE(obj); + MachineClass *mc = MACHINE_GET_CLASS(obj); + SMPTestData data = {}; + unsigned int num_drawers = 5, num_books = 3; + int i; + + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { + data = data_generic_valid[i]; + unsupported_params_init(mc, &data); + + /* + * when drawers and books parameters are omitted, they will + * be both set as 1. + */ + data.expect_prefer_sockets.drawers = 1; + data.expect_prefer_sockets.books = 1; + data.expect_prefer_cores.drawers = 1; + data.expect_prefer_cores.books = 1; + + smp_parse_test(ms, &data, true); + + /* when drawers and books parameters are both specified */ + data.config.has_drawers = true; + data.config.drawers = num_drawers; + data.config.has_books = true; + data.config.books = num_books; + + if (data.config.has_cpus) { + data.config.cpus *= num_drawers * num_books; + } + if (data.config.has_maxcpus) { + data.config.maxcpus *= num_drawers * num_books; + } + + data.expect_prefer_sockets.drawers = num_drawers; + data.expect_prefer_sockets.books = num_books; + data.expect_prefer_sockets.cpus *= num_drawers * num_books; + data.expect_prefer_sockets.max_cpus *= num_drawers * num_books; + + data.expect_prefer_cores.drawers = num_drawers; + data.expect_prefer_cores.books = num_books; + data.expect_prefer_cores.cpus *= num_drawers * num_books; + data.expect_prefer_cores.max_cpus *= num_drawers * num_books; + + smp_parse_test(ms, &data, true); + } + + for (i = 0; i < ARRAY_SIZE(data_with_drawers_books_invalid); i++) { + data = data_with_drawers_books_invalid[i]; + unsupported_params_init(mc, &data); + + smp_parse_test(ms, &data, false); + } + + object_unref(obj); +} + /* Type info of the tested machine */ static const TypeInfo smp_machine_types[] = { { @@ -968,6 +1064,10 @@ static const TypeInfo smp_machine_types[] = { .name = MACHINE_TYPE_NAME("smp-with-drawers"), .parent = TYPE_MACHINE, .class_init = machine_with_drawers_class_init, + }, { + .name = MACHINE_TYPE_NAME("smp-with-drawers-books"), + .parent = TYPE_MACHINE, + .class_init = machine_with_drawers_books_class_init, } }; @@ -997,6 +1097,9 @@ int main(int argc, char *argv[]) g_test_add_data_func("/test-smp-parse/with_drawers", MACHINE_TYPE_NAME("smp-with-drawers"), test_with_drawers); + g_test_add_data_func("/test-smp-parse/with_drawers_books", + MACHINE_TYPE_NAME("smp-with-drawers-books"), + test_with_drawers_books); g_test_run(); From ef88e1e875289aaa5e6645fc0038fa1c82b750e2 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:46 +0800 Subject: [PATCH 40/43] tests/unit/test-smp-parse: Test the full 7-levels topology hierarchy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, -smp supports up to 7-levels topology hierarchy: -drawers/books/sockets/dies/clusters/cores/threads. Though no machine supports all these 7 levels yet, these 7 levels have the strict containment relationship and together form the generic CPU topology representation of QEMU. Also, note that the maxcpus is calculated by multiplying all 7 levels: maxcpus = drawers * books * sockets * dies * clusters * cores * threads. To cover this code path, it is necessary to test the full topology case (with all 7 levels). This also helps to avoid introducing new issues by further expanding the CPU topology in the future. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Acked-by: Thomas Huth Message-ID: <20240308160148.3130837-12-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 143 ++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 0cf6115198..7558169171 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -91,6 +91,24 @@ .has_maxcpus = hg, .maxcpus = g, \ } +/* + * Currently QEMU supports up to a 7-level topology hierarchy, which is the + * QEMU's unified abstract representation of CPU topology. + * -drawers/books/sockets/dies/clusters/cores/threads + */ +#define SMP_CONFIG_WITH_FULL_TOPO(a, b, c, d, e, f, g, h, i) \ + { \ + .has_cpus = true, .cpus = a, \ + .has_drawers = true, .drawers = b, \ + .has_books = true, .books = c, \ + .has_sockets = true, .sockets = d, \ + .has_dies = true, .dies = e, \ + .has_clusters = true, .clusters = f, \ + .has_cores = true, .cores = g, \ + .has_threads = true, .threads = h, \ + .has_maxcpus = true, .maxcpus = i, \ + } + /** * @config - the given SMP configuration * @expect_prefer_sockets - the expected parsing result for the @@ -472,6 +490,40 @@ static const struct SMPTestData data_with_drawers_books_invalid[] = { }, }; +static const struct SMPTestData data_full_topo_invalid[] = { + { + /* + * config: -smp 200,drawers=3,books=5,sockets=2,dies=4,\ + * clusters=2,cores=7,threads=2,maxcpus=200 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(200, 3, 5, 2, 4, 2, 7, 2, 200), + .expect_error = "Invalid CPU topology: " + "product of the hierarchy must match maxcpus: " + "drawers (3) * books (5) * sockets (2) * dies (4) * " + "clusters (2) * cores (7) * threads (2) " + "!= maxcpus (200)", + }, { + /* + * config: -smp 3361,drawers=3,books=5,sockets=2,dies=4,\ + * clusters=2,cores=7,threads=2,maxcpus=3360 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 2, 3360), + .expect_error = "Invalid CPU topology: " + "maxcpus must be equal to or greater than smp: " + "drawers (3) * books (5) * sockets (2) * dies (4) * " + "clusters (2) * cores (7) * threads (2) " + "== maxcpus (3360) < smp_cpus (3361)", + }, { + /* + * config: -smp 1,drawers=3,books=5,sockets=2,dies=4,\ + * clusters=2,cores=7,threads=3,maxcpus=5040 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 3, 5040), + .expect_error = "Invalid SMP CPUs 5040. The max CPUs supported " + "by machine '" SMP_MACHINE_NAME "' is 4096", + }, +}; + static char *smp_config_to_string(const SMPConfiguration *config) { return g_strdup_printf( @@ -733,6 +785,16 @@ static void machine_with_drawers_books_class_init(ObjectClass *oc, void *data) mc->smp_props.books_supported = true; } +static void machine_full_topo_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->smp_props.drawers_supported = true; + mc->smp_props.books_supported = true; + mc->smp_props.dies_supported = true; + mc->smp_props.clusters_supported = true; +} + static void test_generic_valid(const void *opaque) { const char *machine_type = opaque; @@ -1032,6 +1094,80 @@ static void test_with_drawers_books(const void *opaque) object_unref(obj); } +static void test_full_topo(const void *opaque) +{ + const char *machine_type = opaque; + Object *obj = object_new(machine_type); + MachineState *ms = MACHINE(obj); + MachineClass *mc = MACHINE_GET_CLASS(obj); + SMPTestData data = {}; + unsigned int drawers = 5, books = 3, dies = 2, clusters = 7, multiplier; + int i; + + multiplier = drawers * books * dies * clusters; + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { + data = data_generic_valid[i]; + unsupported_params_init(mc, &data); + + /* + * when drawers, books, dies and clusters parameters are omitted, + * they will be set as 1. + */ + data.expect_prefer_sockets.drawers = 1; + data.expect_prefer_sockets.books = 1; + data.expect_prefer_sockets.dies = 1; + data.expect_prefer_sockets.clusters = 1; + data.expect_prefer_cores.drawers = 1; + data.expect_prefer_cores.books = 1; + data.expect_prefer_cores.dies = 1; + data.expect_prefer_cores.clusters = 1; + + smp_parse_test(ms, &data, true); + + /* when drawers, books, dies and clusters parameters are specified. */ + data.config.has_drawers = true; + data.config.drawers = drawers; + data.config.has_books = true; + data.config.books = books; + data.config.has_dies = true; + data.config.dies = dies; + data.config.has_clusters = true; + data.config.clusters = clusters; + + if (data.config.has_cpus) { + data.config.cpus *= multiplier; + } + if (data.config.has_maxcpus) { + data.config.maxcpus *= multiplier; + } + + data.expect_prefer_sockets.drawers = drawers; + data.expect_prefer_sockets.books = books; + data.expect_prefer_sockets.dies = dies; + data.expect_prefer_sockets.clusters = clusters; + data.expect_prefer_sockets.cpus *= multiplier; + data.expect_prefer_sockets.max_cpus *= multiplier; + + data.expect_prefer_cores.drawers = drawers; + data.expect_prefer_cores.books = books; + data.expect_prefer_cores.dies = dies; + data.expect_prefer_cores.clusters = clusters; + data.expect_prefer_cores.cpus *= multiplier; + data.expect_prefer_cores.max_cpus *= multiplier; + + smp_parse_test(ms, &data, true); + } + + for (i = 0; i < ARRAY_SIZE(data_full_topo_invalid); i++) { + data = data_full_topo_invalid[i]; + unsupported_params_init(mc, &data); + + smp_parse_test(ms, &data, false); + } + + object_unref(obj); +} + /* Type info of the tested machine */ static const TypeInfo smp_machine_types[] = { { @@ -1068,6 +1204,10 @@ static const TypeInfo smp_machine_types[] = { .name = MACHINE_TYPE_NAME("smp-with-drawers-books"), .parent = TYPE_MACHINE, .class_init = machine_with_drawers_books_class_init, + }, { + .name = MACHINE_TYPE_NAME("smp-full-topo"), + .parent = TYPE_MACHINE, + .class_init = machine_full_topo_class_init, } }; @@ -1100,6 +1240,9 @@ int main(int argc, char *argv[]) g_test_add_data_func("/test-smp-parse/with_drawers_books", MACHINE_TYPE_NAME("smp-with-drawers-books"), test_with_drawers_books); + g_test_add_data_func("/test-smp-parse/full", + MACHINE_TYPE_NAME("smp-full-topo"), + test_full_topo); g_test_run(); From 71e44ee0b2e034c88eeb507fb9bab6787f1598b5 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:47 +0800 Subject: [PATCH 41/43] tests/unit/test-smp-parse: Test smp_props.has_clusters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The smp_props.has_clusters in MachineClass is not a user configured field, and it indicates if user specifies "clusters" in -smp. After -smp parsing, other module could aware if the cluster level is configured by user. This is used when the machine has only 1 cluster since there's only 1 cluster by default. Add the check to cover this field. Signed-off-by: Zhao Liu Tested-by: Xiaoling Song Acked-by: Thomas Huth Message-ID: <20240308160148.3130837-13-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 7558169171..d39cfdc19b 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -573,7 +573,8 @@ static unsigned int cpu_topology_get_cores_per_socket(const CpuTopology *topo) static char *cpu_topology_to_string(const CpuTopology *topo, unsigned int threads_per_socket, - unsigned int cores_per_socket) + unsigned int cores_per_socket, + bool has_clusters) { return g_strdup_printf( "(CpuTopology) {\n" @@ -588,17 +589,20 @@ static char *cpu_topology_to_string(const CpuTopology *topo, " .max_cpus = %u,\n" " .threads_per_socket = %u,\n" " .cores_per_socket = %u,\n" + " .has_clusters = %s,\n" "}", topo->cpus, topo->drawers, topo->books, topo->sockets, topo->dies, topo->clusters, topo->cores, topo->threads, topo->max_cpus, - threads_per_socket, cores_per_socket); + threads_per_socket, cores_per_socket, + has_clusters ? "true" : "false"); } static void check_parse(MachineState *ms, const SMPConfiguration *config, const CpuTopology *expect_topo, const char *expect_err, bool is_valid) { + MachineClass *mc = MACHINE_GET_CLASS(ms); g_autofree char *config_str = smp_config_to_string(config); g_autofree char *expect_topo_str = NULL, *output_topo_str = NULL; unsigned int expect_threads_per_socket, expect_cores_per_socket; @@ -611,15 +615,18 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config, cpu_topology_get_cores_per_socket(expect_topo); expect_topo_str = cpu_topology_to_string(expect_topo, expect_threads_per_socket, - expect_cores_per_socket); + expect_cores_per_socket, + config->has_clusters); /* call the generic parser */ machine_parse_smp_config(ms, config, &err); ms_threads_per_socket = machine_topo_get_threads_per_socket(ms); ms_cores_per_socket = machine_topo_get_cores_per_socket(ms); - output_topo_str = cpu_topology_to_string(&ms->smp, ms_threads_per_socket, - ms_cores_per_socket); + output_topo_str = cpu_topology_to_string(&ms->smp, + ms_threads_per_socket, + ms_cores_per_socket, + mc->smp_props.has_clusters); /* when the configuration is supposed to be valid */ if (is_valid) { @@ -634,7 +641,8 @@ static void check_parse(MachineState *ms, const SMPConfiguration *config, (ms->smp.threads == expect_topo->threads) && (ms->smp.max_cpus == expect_topo->max_cpus) && (ms_threads_per_socket == expect_threads_per_socket) && - (ms_cores_per_socket == expect_cores_per_socket)) { + (ms_cores_per_socket == expect_cores_per_socket) && + (mc->smp_props.has_clusters == config->has_clusters)) { return; } From bb829cdeffd68195259e7efeb9eb47f8b93c1aab Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sat, 9 Mar 2024 00:01:48 +0800 Subject: [PATCH 42/43] tests/unit/test-smp-parse: Test "parameter=0" SMP configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The support for "parameter=0" SMP configurations is removed, and QEMU returns error for those cases. So add the related test cases to ensure parameters can't accept 0. Signed-off-by: Zhao Liu Reviewed-by: Thomas Huth Message-ID: <20240308160148.3130837-14-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 92 +++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index d39cfdc19b..8994337e12 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -524,6 +524,91 @@ static const struct SMPTestData data_full_topo_invalid[] = { }, }; +static const struct SMPTestData data_zero_topo_invalid[] = { + { + /* + * Test "cpus=0". + * config: -smp 0,drawers=1,books=1,sockets=1,dies=1,\ + * clusters=1,cores=1,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(0, 1, 1, 1, 1, 1, 1, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "drawers=0". + * config: -smp 1,drawers=0,books=1,sockets=1,dies=1,\ + * clusters=1,cores=1,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 0, 1, 1, 1, 1, 1, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "books=0". + * config: -smp 1,drawers=1,books=0,sockets=1,dies=1,\ + * clusters=1,cores=1,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 0, 1, 1, 1, 1, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "sockets=0". + * config: -smp 1,drawers=1,books=1,sockets=0,dies=1,\ + * clusters=1,cores=1,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 0, 1, 1, 1, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "dies=0". + * config: -smp 1,drawers=1,books=1,sockets=1,dies=0,\ + * clusters=1,cores=1,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 0, 1, 1, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "clusters=0". + * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\ + * clusters=0,cores=1,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 0, 1, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "cores=0". + * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\ + * clusters=1,cores=0,threads=1,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 1, 0, 1, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "threads=0". + * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\ + * clusters=1,cores=1,threads=0,maxcpus=1 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 1, 1, 0, 1), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, { + /* + * Test "maxcpus=0". + * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\ + * clusters=1,cores=1,threads=1,maxcpus=0 + */ + .config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 1, 1, 1, 0), + .expect_error = "Invalid CPU topology: CPU topology parameters must " + "be greater than zero", + }, +}; + static char *smp_config_to_string(const SMPConfiguration *config) { return g_strdup_printf( @@ -1173,6 +1258,13 @@ static void test_full_topo(const void *opaque) smp_parse_test(ms, &data, false); } + for (i = 0; i < ARRAY_SIZE(data_zero_topo_invalid); i++) { + data = data_zero_topo_invalid[i]; + unsupported_params_init(mc, &data); + + smp_parse_test(ms, &data, false); + } + object_unref(obj); } From d3c79c397484ad117063702e6246e39f22f020f6 Mon Sep 17 00:00:00 2001 From: Angelo Dureghello Date: Sat, 9 Mar 2024 10:34:59 +0100 Subject: [PATCH 43/43] hw/m68k/mcf5208: add support for reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add reset support for mcf5208. Signed-off-by: Angelo Dureghello Reviewed-by: Thomas Huth Message-ID: <20240309093459.984565-1-angelo@kernel-space.org> Signed-off-by: Philippe Mathieu-Daudé --- hw/m68k/mcf5208.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c index 0cfb806c20..ec14096aa4 100644 --- a/hw/m68k/mcf5208.c +++ b/hw/m68k/mcf5208.c @@ -40,6 +40,8 @@ #define PCSR_PRE_SHIFT 8 #define PCSR_PRE_MASK 0x0f00 +#define RCR_SOFTRST 0x80 + typedef struct { MemoryRegion iomem; qemu_irq irq; @@ -185,12 +187,50 @@ static const MemoryRegionOps m5208_sys_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic) +static uint64_t m5208_rcm_read(void *opaque, hwaddr addr, + unsigned size) +{ + return 0; +} + +static void m5208_rcm_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size) +{ + M68kCPU *cpu = opaque; + CPUState *cs = CPU(cpu); + switch (addr) { + case 0x0: /* RCR */ + if (value & RCR_SOFTRST) { + cpu_reset(cs); + cpu->env.aregs[7] = ldl_phys(cs->as, 0); + cpu->env.pc = ldl_phys(cs->as, 4); + } + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n", + __func__, addr); + break; + } +} + +static const MemoryRegionOps m5208_rcm_ops = { + .read = m5208_rcm_read, + .write = m5208_rcm_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic, + M68kCPU *cpu) { MemoryRegion *iomem = g_new(MemoryRegion, 1); + MemoryRegion *iomem_rcm = g_new(MemoryRegion, 1); m5208_timer_state *s; int i; + /* RCM */ + memory_region_init_io(iomem_rcm, NULL, &m5208_rcm_ops, cpu, + "m5208-rcm", 0x00000080); + memory_region_add_subregion(address_space, 0xfc0a0000, iomem_rcm); /* SDRAMC. */ memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL, "m5208-sys", 0x00004000); memory_region_add_subregion(address_space, 0xfc0a8000, iomem); @@ -265,7 +305,7 @@ static void mcf5208evb_init(MachineState *machine) mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1)); mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2)); - mcf5208_sys_init(address_space_mem, pic); + mcf5208_sys_init(address_space_mem, pic, cpu); mcf_fec_init(address_space_mem, 0xfc030000, pic + 36);