From dcebbb65b8a423a4e933ac803cd27ec8dc03ce7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 22 Mar 2022 13:05:22 +0100 Subject: [PATCH 1/7] target/i386/kvm: Free xsave_buf when destroying vCPU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix vCPU hot-unplug related leak reported by Valgrind: ==132362== 4,096 bytes in 1 blocks are definitely lost in loss record 8,440 of 8,549 ==132362== at 0x4C3B15F: memalign (vg_replace_malloc.c:1265) ==132362== by 0x4C3B288: posix_memalign (vg_replace_malloc.c:1429) ==132362== by 0xB41195: qemu_try_memalign (memalign.c:53) ==132362== by 0xB41204: qemu_memalign (memalign.c:73) ==132362== by 0x7131CB: kvm_init_xsave (kvm.c:1601) ==132362== by 0x7148ED: kvm_arch_init_vcpu (kvm.c:2031) ==132362== by 0x91D224: kvm_init_vcpu (kvm-all.c:516) ==132362== by 0x9242C9: kvm_vcpu_thread_fn (kvm-accel-ops.c:40) ==132362== by 0xB2EB26: qemu_thread_start (qemu-thread-posix.c:556) ==132362== by 0x7EB2159: start_thread (in /usr/lib64/libpthread-2.28.so) ==132362== by 0x9D45DD2: clone (in /usr/lib64/libc-2.28.so) Reported-by: Mark Kanda Signed-off-by: Philippe Mathieu-Daudé Tested-by: Mark Kanda Message-Id: <20220322120522.26200-1-philippe.mathieu.daude@gmail.com> Signed-off-by: Paolo Bonzini --- target/i386/kvm/kvm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 06901c2a43..7396b430d7 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -2081,6 +2081,8 @@ int kvm_arch_destroy_vcpu(CPUState *cs) X86CPU *cpu = X86_CPU(cs); CPUX86State *env = &cpu->env; + g_free(env->xsave_buf); + if (cpu->kvm_msr_buf) { g_free(cpu->kvm_msr_buf); cpu->kvm_msr_buf = NULL; From cb48748af7dfd7654323eb839d1f853ffa873652 Mon Sep 17 00:00:00 2001 From: luofei Date: Thu, 20 Jan 2022 03:46:34 -0500 Subject: [PATCH 2/7] i386: Set MCG_STATUS_RIPV bit for mce SRAR error In the physical machine environment, when a SRAR error occurs, the IA32_MCG_STATUS RIPV bit is set, but qemu does not set this bit. When qemu injects an SRAR error into virtual machine, the virtual machine kernel just call do_machine_check() to kill the current task, but not call memory_failure() to isolate the faulty page, which will cause the faulty page to be allocated and used repeatedly. If used by the virtual machine kernel, it will cause the virtual machine to crash Signed-off-by: luofei Message-Id: <20220120084634.131450-1-luofei@unicloud.com> Signed-off-by: Paolo Bonzini --- target/i386/kvm/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 7396b430d7..9cf8e03669 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -569,7 +569,7 @@ static void kvm_mce_inject(X86CPU *cpu, hwaddr paddr, int code) if (code == BUS_MCEERR_AR) { status |= MCI_STATUS_AR | 0x134; - mcg_status |= MCG_STATUS_EIPV; + mcg_status |= MCG_STATUS_RIPV | MCG_STATUS_EIPV; } else { status |= 0xc0; mcg_status |= MCG_STATUS_RIPV; From 58f7db26f21c690cf9a669c314cfd7371506084a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 23 Mar 2022 12:33:25 +0100 Subject: [PATCH 3/7] KVM: x86: workaround invalid CPUID[0xD,9] info on some AMD processors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some AMD processors expose the PKRU extended save state even if they do not have the related PKU feature in CPUID. Worse, when they do they report a size of 64, whereas the expected size of the PKRU extended save state is 8, therefore the esa->size == eax assertion does not hold. The state is already ignored by KVM_GET_SUPPORTED_CPUID because it was not enabled in the host XCR0. However, QEMU kvm_cpu_xsave_init() runs before QEMU invokes arch_prctl() to enable dynamically-enabled save states such as XTILEDATA, and KVM_GET_SUPPORTED_CPUID hides save states that have yet to be enabled. Therefore, kvm_cpu_xsave_init() needs to consult the host CPUID instead of KVM_GET_SUPPORTED_CPUID, and dies with an assertion failure. When setting up the ExtSaveArea array to match the host, ignore features that KVM does not report as supported. This will cause QEMU to skip the incorrect CPUID leaf instead of tripping the assertion. Closes: https://gitlab.com/qemu-project/qemu/-/issues/916 Reported-by: Daniel P. Berrangé Analyzed-by: Yang Zhong Reported-by: Peter Krempa Tested-by: Daniel P. Berrangé Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 4 ++-- target/i386/cpu.h | 2 ++ target/i386/kvm/kvm-cpu.c | 19 ++++++++++++------- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index a88d6554c8..ec3b50bf6e 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4981,8 +4981,8 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return cpu_list; } -static uint64_t x86_cpu_get_supported_feature_word(FeatureWord w, - bool migratable_only) +uint64_t x86_cpu_get_supported_feature_word(FeatureWord w, + bool migratable_only) { FeatureWordInfo *wi = &feature_word_info[w]; uint64_t r = 0; diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 5e406088a9..e31e6bd8b8 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -606,6 +606,8 @@ typedef enum FeatureWord { } FeatureWord; typedef uint64_t FeatureWordArray[FEATURE_WORDS]; +uint64_t x86_cpu_get_supported_feature_word(FeatureWord w, + bool migratable_only); /* cpuid_features bits */ #define CPUID_FP87 (1U << 0) diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c index a35a1bf9fe..5eb955ce9a 100644 --- a/target/i386/kvm/kvm-cpu.c +++ b/target/i386/kvm/kvm-cpu.c @@ -99,13 +99,18 @@ static void kvm_cpu_xsave_init(void) for (i = XSTATE_SSE_BIT + 1; i < XSAVE_STATE_AREA_COUNT; i++) { ExtSaveArea *esa = &x86_ext_save_areas[i]; - if (esa->size) { - host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx); - if (eax != 0) { - assert(esa->size == eax); - esa->offset = ebx; - esa->ecx = ecx; - } + if (!esa->size) { + continue; + } + if ((x86_cpu_get_supported_feature_word(esa->feature, false) & esa->bits) + != esa->bits) { + continue; + } + host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx); + if (eax != 0) { + assert(esa->size == eax); + esa->offset = ebx; + esa->ecx = ecx; } } } From 98a02bc4492c7e00a703ad42f1ff9d2f3521a138 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 23 Mar 2022 12:46:53 +0100 Subject: [PATCH 4/7] configure: remove dead int128 test Signed-off-by: Paolo Bonzini --- configure | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/configure b/configure index 6d9cb23ac5..7c08c18358 100755 --- a/configure +++ b/configure @@ -2463,24 +2463,6 @@ else # "$safe_stack" = "" fi fi -######################################## -# check if __[u]int128_t is usable. - -int128=no -cat > $TMPC << EOF -__int128_t a; -__uint128_t b; -int main (void) { - a = a + b; - b = a * b; - a = a * a; - return 0; -} -EOF -if compile_prog "" "" ; then - int128=yes -fi - ######################################## # check if ccache is interfering with # semantic analysis of macros From de65b39a517c9977769c612af716dc418ce2ea0b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 24 Mar 2022 09:08:39 +0100 Subject: [PATCH 5/7] target/i386: tcg: high bits SSE cmp operation must be ignored High bits in the immediate operand of SSE comparisons are ignored, they do not result in an undefined opcode exception. This is mentioned explicitly in the Intel documentation. Reported-by: sonicadvance1@gmail.com Closes: https://gitlab.com/qemu-project/qemu/-/issues/184 Signed-off-by: Paolo Bonzini --- target/i386/tcg/translate.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 2a94d33742..c393913fe0 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -4509,10 +4509,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, sse_fn_ppi(s->ptr0, s->ptr1, tcg_const_i32(val)); break; case 0xc2: - /* compare insns */ - val = x86_ldub_code(env, s); - if (val >= 8) - goto unknown_op; + /* compare insns, bits 7:3 (7:5 for AVX) are ignored */ + val = x86_ldub_code(env, s) & 7; sse_fn_epp = sse_op_table4[val][b1]; tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset); From 5286c3662294119dc2dd1e9296757337211451f6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 24 Mar 2022 09:21:41 +0100 Subject: [PATCH 6/7] target/i386: properly reset TSC on reset Some versions of Windows hang on reboot if their TSC value is greater than 2^54. The calibration of the Hyper-V reference time overflows and fails; as a result the processors' clock sources are out of sync. The issue is that the TSC _should_ be reset to 0 on CPU reset and QEMU tries to do that. However, KVM special cases writing 0 to the TSC and thinks that QEMU is trying to hot-plug a CPU, which is correct the first time through but not later. Thwart this valiant effort and reset the TSC to 1 instead, but only if the CPU has been run once. For this to work, env->tsc has to be moved to the part of CPUArchState that is not zeroed at the beginning of x86_cpu_reset. Reported-by: Vadim Rozenfeld Supersedes: <20220324082346.72180-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 13 +++++++++++++ target/i386/cpu.h | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index ec3b50bf6e..cb6b5467d0 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5931,6 +5931,19 @@ static void x86_cpu_reset(DeviceState *dev) env->xstate_bv = 0; env->pat = 0x0007040600070406ULL; + + if (kvm_enabled()) { + /* + * KVM handles TSC = 0 specially and thinks we are hot-plugging + * a new CPU, use 1 instead to force a reset. + */ + if (env->tsc != 0) { + env->tsc = 1; + } + } else { + env->tsc = 0; + } + env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT; if (env->features[FEAT_1_ECX] & CPUID_EXT_MONITOR) { env->msr_ia32_misc_enable |= MSR_IA32_MISC_ENABLE_MWAIT; diff --git a/target/i386/cpu.h b/target/i386/cpu.h index e31e6bd8b8..982c532353 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1554,7 +1554,6 @@ typedef struct CPUArchState { target_ulong kernelgsbase; #endif - uint64_t tsc; uint64_t tsc_adjust; uint64_t tsc_deadline; uint64_t tsc_aux; @@ -1708,6 +1707,7 @@ typedef struct CPUArchState { int64_t tsc_khz; int64_t user_tsc_khz; /* for sanity check only */ uint64_t apic_bus_freq; + uint64_t tsc; #if defined(CONFIG_KVM) || defined(CONFIG_HVF) void *xsave_buf; uint32_t xsave_buf_len; From 9584d3d00a454f47b0341465142bcf0735d734ae Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Wed, 23 Mar 2022 10:07:13 +0100 Subject: [PATCH 7/7] build: disable fcf-protection on -march=486 -m16 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of the roms build with -march=i486 -m16 which is incompatible with -fcf-protection. That in turn is can be set by default, for example in Ubuntu [1]. That causes: cc1: error: ‘-fcf-protection’ is not compatible with this target This won't work on -march=i486 -m16 and no matter if set or not we can override it to "none" if the option is known to the compiler to be able to build reliably. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/889 [1]: https://wiki.ubuntu.com/ToolChain/CompilerFlags#A-fcf-protection Signed-off-by: Christian Ehrhardt Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Message-Id: <20220323090713.1002588-1-christian.ehrhardt@canonical.com> Signed-off-by: Paolo Bonzini --- pc-bios/optionrom/Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile index 5d55d25acc..f1ef898073 100644 --- a/pc-bios/optionrom/Makefile +++ b/pc-bios/optionrom/Makefile @@ -14,6 +14,10 @@ cc-option = $(if $(shell $(CC) $1 -c -o /dev/null -xc /dev/null >/dev/null 2>&1 override CFLAGS += -march=i486 -Wall +# If -fcf-protection is enabled in flags or compiler defaults that will +# conflict with -march=i486 +override CFLAGS += $(call cc-option, -fcf-protection=none) + # Flags for dependency generation override CPPFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d