i386: reorder call to cpu_exec_realizefn

i386 realizefn code is sensitive to ordering, and recent commits
aimed at refactoring it, splitting accelerator-specific code,
broke assumptions which need to be fixed.

We need to:

* process hyper-v enlightements first, as they assume features
  not to be expanded

* only then, expand features

* after expanding features, attempt to check them and modify them in the
  accel-specific realizefn code called by cpu_exec_realizefn().

* after the framework has been called via cpu_exec_realizefn,
  the code can check for what has or hasn't been set by accel-specific
  code, or extend its results, ie:

  - check and evenually set code_urev default
  - modify cpu->mwait after potentially being set from host CPUID.
  - finally check for phys_bits assuming all user and accel-specific
    adjustments have already been taken into account.

Fixes: f5cc5a5c ("i386: split cpu accelerators from cpu.c"...)
Fixes: 30565f10 ("cpu: call AccelCPUClass::cpu_realizefn in"...)
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Claudio Fontana <cfontana@suse.de>
Message-Id: <20210603123001.17843-2-cfontana@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Claudio Fontana 2021-06-03 14:30:00 +02:00 committed by Paolo Bonzini
parent 6b731a96aa
commit 662175b91f
2 changed files with 61 additions and 30 deletions

View File

@ -6089,39 +6089,17 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
Error *local_err = NULL;
static bool ht_warned;
/* Process Hyper-V enlightenments */
x86_cpu_hyperv_realize(cpu);
cpu_exec_realizefn(cs, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
return;
}
if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
g_autofree char *name = x86_cpu_class_get_model_name(xcc);
error_setg(&local_err, "CPU model '%s' requires KVM or HVF", name);
goto out;
}
if (cpu->ucode_rev == 0) {
/* The default is the same as KVM's. */
if (IS_AMD_CPU(env)) {
cpu->ucode_rev = 0x01000065;
} else {
cpu->ucode_rev = 0x100000000ULL;
}
}
/* mwait extended info: needed for Core compatibility */
/* We always wake on interrupt even if host does not have the capability */
cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
if (cpu->apic_id == UNASSIGNED_APIC_ID) {
error_setg(errp, "apic-id property was not initialized properly");
return;
}
/*
* Process Hyper-V enlightenments.
* Note: this currently has to happen before the expansion of CPU features.
*/
x86_cpu_hyperv_realize(cpu);
x86_cpu_expand_features(cpu, &local_err);
if (local_err) {
goto out;
@ -6146,11 +6124,56 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
& CPUID_EXT2_AMD_ALIASES);
}
/*
* note: the call to the framework needs to happen after feature expansion,
* but before the checks/modifications to ucode_rev, mwait, phys_bits.
* These may be set by the accel-specific code,
* and the results are subsequently checked / assumed in this function.
*/
cpu_exec_realizefn(cs, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
return;
}
if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
g_autofree char *name = x86_cpu_class_get_model_name(xcc);
error_setg(&local_err, "CPU model '%s' requires KVM or HVF", name);
goto out;
}
if (cpu->ucode_rev == 0) {
/*
* The default is the same as KVM's. Note that this check
* needs to happen after the evenual setting of ucode_rev in
* accel-specific code in cpu_exec_realizefn.
*/
if (IS_AMD_CPU(env)) {
cpu->ucode_rev = 0x01000065;
} else {
cpu->ucode_rev = 0x100000000ULL;
}
}
/*
* mwait extended info: needed for Core compatibility
* We always wake on interrupt even if host does not have the capability.
*
* requires the accel-specific code in cpu_exec_realizefn to
* have already acquired the CPUID data into cpu->mwait.
*/
cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
/* For 64bit systems think about the number of physical bits to present.
* ideally this should be the same as the host; anything other than matching
* the host can cause incorrect guest behaviour.
* QEMU used to pick the magic value of 40 bits that corresponds to
* consumer AMD devices but nothing else.
*
* Note that this code assumes features expansion has already been done
* (as it checks for CPUID_EXT2_LM), and also assumes that potential
* phys_bits adjustments to match the host have been already done in
* accel-specific code in cpu_exec_realizefn.
*/
if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
if (cpu->phys_bits &&

View File

@ -26,10 +26,18 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
/*
* The realize order is important, since x86_cpu_realize() checks if
* nothing else has been set by the user (or by accelerators) in
* cpu->ucode_rev and cpu->phys_bits.
* cpu->ucode_rev and cpu->phys_bits, and updates the CPUID results in
* mwait.ecx.
* This accel realization code also assumes cpu features are already expanded.
*
* realize order:
* kvm_cpu -> host_cpu -> x86_cpu
*
* x86_cpu_realize():
* -> x86_cpu_expand_features()
* -> cpu_exec_realizefn():
* -> accel_cpu_realizefn()
* kvm_cpu_realizefn() -> host_cpu_realizefn()
* -> check/update ucode_rev, phys_bits, mwait
*/
if (cpu->max_features) {
if (enable_cpu_pm && kvm_has_waitpkg()) {