From 2cc68629a6fc198f4a972698bdd6477f883aedfb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 11 Mar 2024 08:56:44 +0100 Subject: [PATCH 1/6] target/i386: fix direction of "32-bit MMU" test The low bit of MMU indices for x86 TCG indicates whether the processor is in 32-bit mode and therefore linear addresses have to be masked to 32 bits. However, the index was computed incorrectly, leading to possible conflicts in the TLB for any address above 4G. Analyzed-by: Mark Cave-Ayland Fixes: b1661801c18 ("target/i386: Fix physical address truncation", 2024-02-28) Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2206 Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 2 +- target/i386/cpu.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 9a210d8d92..33760a2ee1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -7735,7 +7735,7 @@ static bool x86_cpu_has_work(CPUState *cs) static int x86_cpu_mmu_index(CPUState *cs, bool ifetch) { CPUX86State *env = cpu_env(cs); - int mmu_index_32 = (env->hflags & HF_CS64_MASK) ? 1 : 0; + int mmu_index_32 = (env->hflags & HF_CS64_MASK) ? 0 : 1; int mmu_index_base = (env->hflags & HF_CPL_MASK) == 3 ? MMU_USER64_IDX : !(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX : diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 952174bb6f..6b05738079 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -2334,7 +2334,7 @@ static inline bool is_mmu_index_32(int mmu_index) static inline int cpu_mmu_index_kernel(CPUX86State *env) { - int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 1 : 0; + int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 0 : 1; int mmu_index_base = !(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX : ((env->hflags & HF_CPL_MASK) < 3 && (env->eflags & AC_MASK)) ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX; From 7ea9cfc8ab57b922822433bc034c484689523c91 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 18 Mar 2024 17:20:01 -0400 Subject: [PATCH 2/6] vl: convert qemu_machine_creation_done() to Error ** Allow using Error ** to pass an error string up to qmp_x_exit_preconfig() and possibly main(). Signed-off-by: Paolo Bonzini --- system/vl.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/system/vl.c b/system/vl.c index 70f4cece7f..0c970cf020 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2653,7 +2653,7 @@ static void qemu_create_cli_devices(void) rom_reset_order_override(); } -static void qemu_machine_creation_done(void) +static bool qemu_machine_creation_done(Error **errp) { MachineState *machine = MACHINE(qdev_get_machine()); @@ -2684,7 +2684,8 @@ static void qemu_machine_creation_done(void) } if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) { - exit(1); + error_setg(errp, "could not start gdbserver"); + return false; } if (!vga_interface_created && !default_vga && vga_interface_type != VGA_NONE) { @@ -2692,6 +2693,7 @@ static void qemu_machine_creation_done(void) "type does not use that option; " "No VGA device has been created"); } + return true; } void qmp_x_exit_preconfig(Error **errp) @@ -2703,7 +2705,9 @@ void qmp_x_exit_preconfig(Error **errp) qemu_init_board(); qemu_create_cli_devices(); - qemu_machine_creation_done(); + if (!qemu_machine_creation_done(errp)) { + return; + } if (loadvm) { RunState state = autostart ? RUN_STATE_RUNNING : runstate_get(); From 3343f296ff9474ce2c3b73ff25b6eece18799216 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 18 Mar 2024 17:20:46 -0400 Subject: [PATCH 3/6] vl: do not assert if sev-guest is used together with TCG Signed-off-by: Paolo Bonzini --- system/vl.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/system/vl.c b/system/vl.c index 0c970cf020..c644222982 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2676,11 +2676,10 @@ static bool qemu_machine_creation_done(Error **errp) qdev_machine_creation_done(); - if (machine->cgs) { - /* - * Verify that Confidential Guest Support has actually been initialized - */ - assert(machine->cgs->ready); + if (machine->cgs && !machine->cgs->ready) { + error_setg(errp, "accelerator does not support confidential guest %s", + object_get_typename(OBJECT(machine->cgs))); + exit(1); } if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) { From 7fd226b04746f0be0b636de5097f1b42338951a0 Mon Sep 17 00:00:00 2001 From: Tao Su Date: Wed, 20 Mar 2024 16:36:40 +0800 Subject: [PATCH 4/6] target/i386: Revert monitor_puts() in do_inject_x86_mce() monitor_puts() doesn't check the monitor pointer, but do_inject_x86_mce() may have a parameter with NULL monitor pointer. Revert monitor_puts() in do_inject_x86_mce() to fix, then the fact that we send the same message to monitor and log is again more obvious. Fixes: bf0c50d4aa85 (monitor: expose monitor_puts to rest of code) Reviwed-by: Xiaoyao Li Reviewed-by: Markus Armbruster Signed-off-by: Tao Su Message-ID: <20240320083640.523287-1-tao1.su@linux.intel.com> Signed-off-by: Paolo Bonzini --- target/i386/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/helper.c b/target/i386/helper.c index 2070dd0dda..23ccb23a5b 100644 --- a/target/i386/helper.c +++ b/target/i386/helper.c @@ -430,7 +430,7 @@ static void do_inject_x86_mce(CPUState *cs, run_on_cpu_data data) if (need_reset) { emit_guest_memory_failure(MEMORY_FAILURE_ACTION_RESET, ar, recursive); - monitor_puts(params->mon, msg); + monitor_printf(params->mon, "%s", msg); qemu_log_mask(CPU_LOG_RESET, "%s\n", msg); qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); return; From 732810235f8e7c0f7c961201d9d05b1f5c1ab5a5 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 11 Mar 2024 09:16:12 +0100 Subject: [PATCH 5/6] tests/plugins: fix use-after-free bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rec->count.score is inside rec, which is freed before rec->count.score is. Reorder the instructions Reported by Coverity as CID 1539967. Cc: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- contrib/plugins/howvec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c index 2d10c87e0f..94bbc53820 100644 --- a/contrib/plugins/howvec.c +++ b/contrib/plugins/howvec.c @@ -167,9 +167,9 @@ static gint cmp_exec_count(gconstpointer a, gconstpointer b) static void free_record(gpointer data) { InsnExecCount *rec = (InsnExecCount *) data; + qemu_plugin_scoreboard_free(rec->count.score); g_free(rec->insn); g_free(rec); - qemu_plugin_scoreboard_free(rec->count.score); } static void plugin_exit(qemu_plugin_id_t id, void *p) From 05007258f02da253af370387b69fe98e9f37b320 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 20 Mar 2024 11:28:28 +0100 Subject: [PATCH 6/6] meson: remove dead dictionary access The "link_depends" key has not been used since commit c46f76d1586 ("meson: specify fuzz linker script as a project arg", 2020-09-08), and even before that it was only used for fork-fuzzing which we removed in commit d2e6f9272d3 ("fuzz: remove fork-fuzzing scaffolding", 2023-02-16). So, remove it for a very small simplification of meson.build. Signed-off-by: Paolo Bonzini --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index b375248a76..c9c3217ba4 100644 --- a/meson.build +++ b/meson.build @@ -3951,7 +3951,7 @@ foreach target : target_dirs c_args: c_args, dependencies: arch_deps + deps + exe['dependencies'], objects: lib.extract_all_objects(recursive: true), - link_depends: [block_syms, qemu_syms] + exe.get('link_depends', []), + link_depends: [block_syms, qemu_syms], link_args: link_args, win_subsystem: exe['win_subsystem'])