hyperv: fix SynIC SINT assertion failure on guest reset

Resetting a guest that has Hyper-V VMBus support enabled triggers a QEMU
assertion failure:
hw/hyperv/hyperv.c:131: synic_reset: Assertion `QLIST_EMPTY(&synic->sint_routes)' failed.

This happens both on normal guest reboot or when using "system_reset" HMP
command.

The failing assertion was introduced by commit 64ddecc88b ("hyperv: SControl is optional to enable SynIc")
to catch dangling SINT routes on SynIC reset.

The root cause of this problem is that the SynIC itself is reset before
devices using SINT routes have chance to clean up these routes.

Since there seems to be no existing mechanism to force reset callbacks (or
methods) to be executed in specific order let's use a similar method that
is already used to reset another interrupt controller (APIC) after devices
have been reset - by invoking the SynIC reset from the machine reset
handler via a new x86_cpu_after_reset() function co-located with
the existing x86_cpu_reset() in target/i386/cpu.c.
Opportunistically move the APIC reset handler there, too.

Fixes: 64ddecc88b ("hyperv: SControl is optional to enable SynIc") # exposed the bug
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Message-Id: <cb57cee2e29b20d06f81dce054cbcea8b5d497e8.1664552976.git.maciej.szmigiero@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Maciej S. Szmigiero 2022-09-30 17:52:03 +02:00 committed by Paolo Bonzini
parent 8b5335e381
commit ec19444a53
7 changed files with 41 additions and 14 deletions

View File

@ -485,9 +485,7 @@ static void microvm_machine_reset(MachineState *machine)
CPU_FOREACH(cs) { CPU_FOREACH(cs) {
cpu = X86_CPU(cs); cpu = X86_CPU(cs);
if (cpu->apic_state) { x86_cpu_after_reset(cpu);
device_legacy_reset(cpu->apic_state);
}
} }
} }

View File

@ -92,6 +92,7 @@
#include "hw/virtio/virtio-mem-pci.h" #include "hw/virtio/virtio-mem-pci.h"
#include "hw/mem/memory-device.h" #include "hw/mem/memory-device.h"
#include "sysemu/replay.h" #include "sysemu/replay.h"
#include "target/i386/cpu.h"
#include "qapi/qmp/qerror.h" #include "qapi/qmp/qerror.h"
#include "e820_memory_layout.h" #include "e820_memory_layout.h"
#include "fw_cfg.h" #include "fw_cfg.h"
@ -1859,9 +1860,7 @@ static void pc_machine_reset(MachineState *machine)
CPU_FOREACH(cs) { CPU_FOREACH(cs) {
cpu = X86_CPU(cs); cpu = X86_CPU(cs);
if (cpu->apic_state) { x86_cpu_after_reset(cpu);
device_legacy_reset(cpu->apic_state);
}
} }
} }

View File

@ -6035,6 +6035,19 @@ static void x86_cpu_reset(DeviceState *dev)
#endif #endif
} }
void x86_cpu_after_reset(X86CPU *cpu)
{
#ifndef CONFIG_USER_ONLY
if (kvm_enabled()) {
kvm_arch_after_reset_vcpu(cpu);
}
if (cpu->apic_state) {
device_legacy_reset(cpu->apic_state);
}
#endif
}
static void mce_init(X86CPU *cpu) static void mce_init(X86CPU *cpu)
{ {
CPUX86State *cenv = &cpu->env; CPUX86State *cenv = &cpu->env;

View File

@ -2082,6 +2082,8 @@ typedef struct PropValue {
} PropValue; } PropValue;
void x86_cpu_apply_props(X86CPU *cpu, PropValue *props); void x86_cpu_apply_props(X86CPU *cpu, PropValue *props);
void x86_cpu_after_reset(X86CPU *cpu);
uint32_t cpu_x86_virtual_addr_width(CPUX86State *env); uint32_t cpu_x86_virtual_addr_width(CPUX86State *env);
/* cpu.c other functions (cpuid) */ /* cpu.c other functions (cpuid) */

View File

@ -23,6 +23,10 @@ int hyperv_x86_synic_add(X86CPU *cpu)
return 0; return 0;
} }
/*
* All devices possibly using SynIC have to be reset before calling this to let
* them remove their SINT routes first.
*/
void hyperv_x86_synic_reset(X86CPU *cpu) void hyperv_x86_synic_reset(X86CPU *cpu)
{ {
hyperv_synic_reset(CPU(cpu)); hyperv_synic_reset(CPU(cpu));

View File

@ -2203,14 +2203,6 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
env->mp_state = KVM_MP_STATE_RUNNABLE; env->mp_state = KVM_MP_STATE_RUNNABLE;
} }
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
int i;
for (i = 0; i < ARRAY_SIZE(env->msr_hv_synic_sint); i++) {
env->msr_hv_synic_sint[i] = HV_SINT_MASKED;
}
hyperv_x86_synic_reset(cpu);
}
/* enabled by default */ /* enabled by default */
env->poll_control_msr = 1; env->poll_control_msr = 1;
@ -2219,6 +2211,24 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
sev_es_set_reset_vector(CPU(cpu)); sev_es_set_reset_vector(CPU(cpu));
} }
void kvm_arch_after_reset_vcpu(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
int i;
/*
* Reset SynIC after all other devices have been reset to let them remove
* their SINT routes first.
*/
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
for (i = 0; i < ARRAY_SIZE(env->msr_hv_synic_sint); i++) {
env->msr_hv_synic_sint[i] = HV_SINT_MASKED;
}
hyperv_x86_synic_reset(cpu);
}
}
void kvm_arch_do_init_vcpu(X86CPU *cpu) void kvm_arch_do_init_vcpu(X86CPU *cpu)
{ {
CPUX86State *env = &cpu->env; CPUX86State *env = &cpu->env;

View File

@ -38,6 +38,7 @@ bool kvm_has_adjust_clock_stable(void);
bool kvm_has_exception_payload(void); bool kvm_has_exception_payload(void);
void kvm_synchronize_all_tsc(void); void kvm_synchronize_all_tsc(void);
void kvm_arch_reset_vcpu(X86CPU *cs); void kvm_arch_reset_vcpu(X86CPU *cs);
void kvm_arch_after_reset_vcpu(X86CPU *cpu);
void kvm_arch_do_init_vcpu(X86CPU *cs); void kvm_arch_do_init_vcpu(X86CPU *cs);
void kvm_put_apicbase(X86CPU *cpu, uint64_t value); void kvm_put_apicbase(X86CPU *cpu, uint64_t value);