From 6d9abb6de9cc53a508823db0283061824f2f98a2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 11 Nov 2020 09:25:29 -0500 Subject: [PATCH 1/8] qom: use qemu_printf to print help for user-creatable objects Since we have added help support for object_add, the help is printed on stdout. Switch to qemu_printf so that it goes to the monitor. Signed-off-by: Paolo Bonzini --- qom/object_interfaces.c | 1 + 1 file changed, 1 insertion(+) diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index 7661270b98..11ad2e2078 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -13,6 +13,7 @@ #include "qemu/option.h" #include "qapi/opts-visitor.h" #include "qemu/config-file.h" +#include "qemu/qemu-print.h" bool user_creatable_complete(UserCreatable *uc, Error **errp) { From b8184135835068e9579e678f0b54465ee6161d32 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Mar 2021 09:35:56 -0400 Subject: [PATCH 2/8] target/i386: allow modifying TCG phys-addr-bits Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 23 ++++++++--------------- target/i386/cpu.h | 1 - target/i386/tcg/excp_helper.c | 7 ++++--- target/i386/tcg/helper-tcg.h | 12 ++++-------- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index ae9fd9f31d..6b3e9467f1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6785,21 +6785,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) cpu->phys_bits = cpu->host_phys_bits_limit; } } - - if (cpu->phys_bits && - (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS || - cpu->phys_bits < 32)) { - error_setg(errp, "phys-bits should be between 32 and %u " - " (but is %u)", - TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits); - return; - } - } else { - if (cpu->phys_bits && cpu->phys_bits != TCG_PHYS_ADDR_BITS) { - error_setg(errp, "TCG only supports phys-bits=%u", - TCG_PHYS_ADDR_BITS); - return; - } + } + if (cpu->phys_bits && + (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS || + cpu->phys_bits < 32)) { + error_setg(errp, "phys-bits should be between 32 and %u " + " (but is %u)", + TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits); + return; } /* 0 means it was not explicitly set by the user (or by machine * compat_props or by the host code above). In this case, the default diff --git a/target/i386/cpu.h b/target/i386/cpu.h index b4b136cd0d..570f916878 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -290,7 +290,6 @@ typedef enum X86Seg { #define PG_GLOBAL_MASK (1 << PG_GLOBAL_BIT) #define PG_PSE_PAT_MASK (1 << PG_PSE_PAT_BIT) #define PG_ADDRESS_MASK 0x000ffffffffff000LL -#define PG_HI_RSVD_MASK (PG_ADDRESS_MASK & ~PHYS_ADDR_MASK) #define PG_HI_USER_MASK 0x7ff0000000000000LL #define PG_PKRU_MASK (15ULL << PG_PKRU_BIT) #define PG_NX_MASK (1ULL << PG_NX_BIT) diff --git a/target/i386/tcg/excp_helper.c b/target/i386/tcg/excp_helper.c index b7d6259e4a..1e71e44510 100644 --- a/target/i386/tcg/excp_helper.c +++ b/target/i386/tcg/excp_helper.c @@ -142,8 +142,9 @@ void raise_exception_ra(CPUX86State *env, int exception_index, uintptr_t retaddr static hwaddr get_hphys(CPUState *cs, hwaddr gphys, MMUAccessType access_type, int *prot) { - CPUX86State *env = &X86_CPU(cs)->env; - uint64_t rsvd_mask = PG_HI_RSVD_MASK; + X86CPU *cpu = X86_CPU(cs); + CPUX86State *env = &cpu->env; + uint64_t rsvd_mask = PG_ADDRESS_MASK & ~MAKE_64BIT_MASK(0, cpu->phys_bits); uint64_t ptep, pte; uint64_t exit_info_1 = 0; target_ulong pde_addr, pte_addr; @@ -358,7 +359,7 @@ static int handle_mmu_fault(CPUState *cs, vaddr addr, int size, int error_code = 0; int is_dirty, prot, page_size, is_write, is_user; hwaddr paddr; - uint64_t rsvd_mask = PG_HI_RSVD_MASK; + uint64_t rsvd_mask = PG_ADDRESS_MASK & ~MAKE_64BIT_MASK(0, cpu->phys_bits); uint32_t page_offset; target_ulong vaddr; uint32_t pkr; diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h index c133c63555..ef60e2e04b 100644 --- a/target/i386/tcg/helper-tcg.h +++ b/target/i386/tcg/helper-tcg.h @@ -25,17 +25,13 @@ /* Maximum instruction code size */ #define TARGET_MAX_INSN_SIZE 16 -/* - * XXX: This value should match the one returned by CPUID - * and in exec.c - */ -# if defined(TARGET_X86_64) +#if defined(TARGET_X86_64) # define TCG_PHYS_ADDR_BITS 40 -# else +#else # define TCG_PHYS_ADDR_BITS 36 -# endif +#endif -#define PHYS_ADDR_MASK MAKE_64BIT_MASK(0, TCG_PHYS_ADDR_BITS) +QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS); /** * x86_cpu_do_interrupt: From da3f3b020f5346f59c30241ff6f3da60be87ed78 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Mar 2021 09:42:32 -0400 Subject: [PATCH 3/8] target/i386: fail if toggling LA57 in 64-bit mode This fixes kvm-unit-tests access.flat with -cpu qemu64,la57. Signed-off-by: Paolo Bonzini --- target/i386/tcg/misc_helper.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/i386/tcg/misc_helper.c b/target/i386/tcg/misc_helper.c index 90b87fdef0..a25428c36e 100644 --- a/target/i386/tcg/misc_helper.c +++ b/target/i386/tcg/misc_helper.c @@ -167,6 +167,10 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0) cpu_x86_update_cr3(env, t0); break; case 4: + if (((t0 ^ env->cr[4]) & CR4_LA57_MASK) && + (env->hflags & HF_CS64_MASK)) { + raise_exception_ra(env, EXCP0D_GPF, GETPC()); + } cpu_x86_update_cr4(env, t0); break; case 8: From 687758565ab39028f7310c30355a765f171ff1bf Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Mar 2021 10:02:06 -0400 Subject: [PATCH 4/8] target/i386: svm: do not discard high 32 bits of EXITINFO1 env->error_code is only 32-bits wide, so the high 32 bits of EXITINFO1 are being lost. However, even though saving guest state and restoring host state must be delayed to do_vmexit, because they might take tb_lock, it is always possible to write to the VMCB. So do this for the exit code and EXITINFO1, just like it is already being done for EXITINFO2. Signed-off-by: Paolo Bonzini --- target/i386/tcg/helper-tcg.h | 2 +- target/i386/tcg/seg_helper.c | 4 ++-- target/i386/tcg/svm_helper.c | 15 +++++++-------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h index ef60e2e04b..bcdfca06f6 100644 --- a/target/i386/tcg/helper-tcg.h +++ b/target/i386/tcg/helper-tcg.h @@ -80,7 +80,7 @@ void cpu_load_eflags(CPUX86State *env, int eflags, int update_mask); /* svm_helper.c */ void QEMU_NORETURN cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1, uintptr_t retaddr); -void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1); +void do_vmexit(CPUX86State *env); /* seg_helper.c */ void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw); diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index 180d47f0e9..d180a381d1 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -1305,9 +1305,9 @@ void x86_cpu_do_interrupt(CPUState *cs) /* successfully delivered */ env->old_exception = -1; #else - if (cs->exception_index >= EXCP_VMEXIT) { + if (cs->exception_index == EXCP_VMEXIT) { assert(env->old_exception == -1); - do_vmexit(env, cs->exception_index - EXCP_VMEXIT, env->error_code); + do_vmexit(env); } else { do_interrupt_all(cpu, cs->exception_index, env->exception_is_int, diff --git a/target/i386/tcg/svm_helper.c b/target/i386/tcg/svm_helper.c index 097bb9b83d..0145afceae 100644 --- a/target/i386/tcg/svm_helper.c +++ b/target/i386/tcg/svm_helper.c @@ -621,15 +621,19 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1, control.exit_info_2)), env->eip); - cs->exception_index = EXCP_VMEXIT + exit_code; - env->error_code = exit_info_1; + cs->exception_index = EXCP_VMEXIT; + x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_code), + exit_code); + + x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, + control.exit_info_1), exit_info_1), /* remove any pending exception */ env->old_exception = -1; cpu_loop_exit(cs); } -void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1) +void do_vmexit(CPUX86State *env) { CPUState *cs = env_cpu(env); uint32_t int_ctl; @@ -762,11 +766,6 @@ void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1) env->vm_hsave + offsetof(struct vmcb, save.dr7)); /* other setups */ - x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_code), - exit_code); - x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_info_1), - exit_info_1); - x86_stl_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info), x86_ldl_phys(cs, env->vm_vmcb + offsetof(struct vmcb, From 816d20c927a930f1ff66656a8dafd9861806c602 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Thu, 18 Mar 2021 17:02:48 +0100 Subject: [PATCH 5/8] i386: Fix 'hypercall_hypercall' typo Even the name of this section is 'cpu/msr_hyperv_hypercall', 'hypercall_hypercall' is clearly a typo. Signed-off-by: Vitaly Kuznetsov Message-Id: <20210318160249.1084178-3-vkuznets@redhat.com> Signed-off-by: Paolo Bonzini --- target/i386/machine.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/i386/machine.c b/target/i386/machine.c index 3967dfc257..7259fe6868 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -705,7 +705,7 @@ static bool hyperv_hypercall_enable_needed(void *opaque) return env->msr_hv_hypercall != 0 || env->msr_hv_guest_os_id != 0; } -static const VMStateDescription vmstate_msr_hypercall_hypercall = { +static const VMStateDescription vmstate_msr_hyperv_hypercall = { .name = "cpu/msr_hyperv_hypercall", .version_id = 1, .minimum_version_id = 1, @@ -1484,7 +1484,7 @@ VMStateDescription vmstate_x86_cpu = { &vmstate_msr_ia32_feature_control, &vmstate_msr_architectural_pmu, &vmstate_mpx, - &vmstate_msr_hypercall_hypercall, + &vmstate_msr_hyperv_hypercall, &vmstate_msr_hyperv_vapic, &vmstate_msr_hyperv_time, &vmstate_msr_hyperv_crash, From 561dbb41b1d752098249128d8462aaadc56fd15d Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Fri, 19 Mar 2021 13:38:01 +0100 Subject: [PATCH 6/8] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset KVM doesn't fully support Hyper-V reenlightenment notifications on migration. In particular, it doesn't support emulating TSC frequency of the source host by trapping all TSC accesses so unless TSC scaling is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it is unsafe to proceed with migration. KVM_SET_TSC_KHZ is called from two sites: kvm_arch_init_vcpu() and kvm_arch_put_registers(). The later (intentionally) doesn't propagate errors allowing migrations to succeed even when TSC scaling is not supported on the destination. This doesn't suit 're-enlightenment' use-case as we have to guarantee that TSC frequency stays constant. Require 'tsc-frequency=' command line option to be specified for successful migration when re-enlightenment was enabled by the guest. Signed-off-by: Vitaly Kuznetsov Message-Id: <20210319123801.1111090-1-vkuznets@redhat.com> Signed-off-by: Paolo Bonzini --- docs/hyperv.txt | 5 +++++ target/i386/kvm/hyperv-proto.h | 1 + target/i386/machine.c | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/docs/hyperv.txt b/docs/hyperv.txt index 5df00da54f..e53c581f45 100644 --- a/docs/hyperv.txt +++ b/docs/hyperv.txt @@ -160,6 +160,11 @@ the hypervisor) until it is ready to switch to the new one. This, in conjunction with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference TSC page) to its own guests. +Note, KVM doesn't fully support re-enlightenment notifications and doesn't +emulate TSC accesses after migration so 'tsc-frequency=' CPU option also has to +be specified to make migration succeed. The destination host has to either have +the same TSC frequency or support TSC scaling CPU feature. + Recommended: hv-frequencies 3.16. hv-evmcs diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h index 056a305be3..e30d64b4ad 100644 --- a/target/i386/kvm/hyperv-proto.h +++ b/target/i386/kvm/hyperv-proto.h @@ -139,6 +139,7 @@ * Reenlightenment notification MSRs */ #define HV_X64_MSR_REENLIGHTENMENT_CONTROL 0x40000106 +#define HV_REENLIGHTENMENT_ENABLE_BIT (1u << 16) #define HV_X64_MSR_TSC_EMULATION_CONTROL 0x40000107 #define HV_X64_MSR_TSC_EMULATION_STATUS 0x40000108 diff --git a/target/i386/machine.c b/target/i386/machine.c index 7259fe6868..137604ddb8 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -883,11 +883,31 @@ static bool hyperv_reenlightenment_enable_needed(void *opaque) env->msr_hv_tsc_emulation_status != 0; } +static int hyperv_reenlightenment_post_load(void *opaque, int version_id) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + + /* + * KVM doesn't fully support re-enlightenment notifications so we need to + * make sure TSC frequency doesn't change upon migration. + */ + if ((env->msr_hv_reenlightenment_control & HV_REENLIGHTENMENT_ENABLE_BIT) && + !env->user_tsc_khz) { + error_report("Guest enabled re-enlightenment notifications, " + "'tsc-frequency=' has to be specified"); + return -EINVAL; + } + + return 0; +} + static const VMStateDescription vmstate_msr_hyperv_reenlightenment = { .name = "cpu/msr_hyperv_reenlightenment", .version_id = 1, .minimum_version_id = 1, .needed = hyperv_reenlightenment_enable_needed, + .post_load = hyperv_reenlightenment_post_load, .fields = (VMStateField[]) { VMSTATE_UINT64(env.msr_hv_reenlightenment_control, X86CPU), VMSTATE_UINT64(env.msr_hv_tsc_emulation_control, X86CPU), From 52ad57a9b68e215c4f5fa2cfd2412132b522bff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 18 Mar 2021 17:30:59 +0100 Subject: [PATCH 7/8] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some compiler versions are smart enough to detect a potentially uninitialized variable, but are not smart enough to detect that this cannot happen due to the code flow: ../hw/intc/i8259.c: In function ‘pic_read_irq’: ../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 203 | irq = irq2 + 8; | ~~~~^~~~~~~~~~ Restrict irq2 variable use to the inner statement. Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq") Reported-by: Christian Borntraeger Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210318163059.3686596-1-philmd@redhat.com> Signed-off-by: Paolo Bonzini --- hw/intc/i8259.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c index 344fd04db1..cc4e21ffec 100644 --- a/hw/intc/i8259.c +++ b/hw/intc/i8259.c @@ -176,10 +176,12 @@ static void pic_intack(PICCommonState *s, int irq) int pic_read_irq(DeviceState *d) { PICCommonState *s = PIC_COMMON(d); - int irq, irq2, intno; + int irq, intno; irq = pic_get_irq(s); if (irq >= 0) { + int irq2; + if (irq == 2) { irq2 = pic_get_irq(slave_pic); if (irq2 >= 0) { @@ -189,20 +191,18 @@ int pic_read_irq(DeviceState *d) irq2 = 7; } intno = slave_pic->irq_base + irq2; + pic_intack(s, irq); + irq = irq2 + 8; } else { intno = s->irq_base + irq; + pic_intack(s, irq); } - pic_intack(s, irq); } else { /* spurious IRQ on host controller */ irq = 7; intno = s->irq_base + irq; } - if (irq == 2) { - irq = irq2 + 8; - } - #ifdef DEBUG_IRQ_LATENCY printf("IRQ%d latency=%0.3fus\n", irq, From af05ffffd7fa28010d4df9f5744514b16e71055d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 19 Mar 2021 04:44:42 -0400 Subject: [PATCH 8/8] tests/qtest: cleanup the testcase for bug 1878642 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean up the writes to the configuration space and the PM region, and rename the test to lpc-ich9-test. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- tests/qtest/{fuzz-test.c => lpc-ich9-test.c} | 12 +++++++----- tests/qtest/meson.build | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) rename tests/qtest/{fuzz-test.c => lpc-ich9-test.c} (71%) diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/lpc-ich9-test.c similarity index 71% rename from tests/qtest/fuzz-test.c rename to tests/qtest/lpc-ich9-test.c index 00149abec7..fe0bef9980 100644 --- a/tests/qtest/fuzz-test.c +++ b/tests/qtest/lpc-ich9-test.c @@ -1,5 +1,5 @@ /* - * QTest testcase for fuzz case + * QTest testcases for ich9 case * * Copyright (c) 2020 Li Qiang * @@ -18,9 +18,11 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void) s = qtest_init("-M pc-q35-5.0 " "-nographic -monitor none -serial none"); - qtest_outl(s, 0xcf8, 0x8400f841); - qtest_outl(s, 0xcfc, 0xebed205d); - qtest_outl(s, 0x5d02, 0xebed205d); + qtest_outl(s, 0xcf8, 0x8000f840); /* PMBASE */ + qtest_outl(s, 0xcfc, 0x5d00); + qtest_outl(s, 0xcf8, 0x8000f844); /* ACPI_CTRL */ + qtest_outl(s, 0xcfc, 0xeb); + qtest_outw(s, 0x5d02, 0x205d); qtest_quit(s); } @@ -31,7 +33,7 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { - qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert", + qtest_add_func("ich9/test_lp1878642_pci_bus_get_irq_level_assert", test_lp1878642_pci_bus_get_irq_level_assert); } diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 6e871077c1..9731606c31 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -57,6 +57,7 @@ qtests_i386 = \ (config_all_devices.has_key('CONFIG_HDA') ? ['intel-hda-test'] : []) + \ (config_all_devices.has_key('CONFIG_I82801B11') ? ['i82801b11-test'] : []) + \ (config_all_devices.has_key('CONFIG_IOH3420') ? ['ioh3420-test'] : []) + \ + (config_all_devices.has_key('CONFIG_LPC_ICH9') ? ['lpc-ich9-test'] : []) + \ (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) + \ (config_all_devices.has_key('CONFIG_USB_UHCI') and \ config_all_devices.has_key('CONFIG_USB_EHCI') ? ['usb-hcd-ehci-test'] : []) + \ @@ -74,7 +75,6 @@ qtests_i386 = \ 'bios-tables-test', 'rtc-test', 'i440fx-test', - 'fuzz-test', 'fw_cfg-test', 'device-plug-test', 'drive_del-test',