properties set by function x86_cpu_apply_props, including
kvm_default_props, tcg_default_props,
and the "vendor" property for KVM and HVF,
are actually to be set only for cpu models in builtin_x86_defs,
registered with x86_register_cpu_model_type, and not for
cpu models "base", "max", and the subclass "host".
This has been detected as a bug with Nested on AMD with cpu "host",
as svm was not turned on by default, due to the wrongful setting of
kvm_default_props via x86_cpu_apply_props.
Rectify the bug introduced in commit "i386: split cpu accelerators"
and document the functions that are builtin_x86_defs-only.
Signed-off-by: Claudio Fontana <cfont...@suse.de>
Tested-by: Alexander Bulekov <alx...@bu.edu>
Fixes: f5cc5a5c ("i386: split cpu accelerators from cpu.c,"...)
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/477
---
target/i386/cpu.c | 19 ++++++-
target/i386/host-cpu.c | 13 +++--
target/i386/kvm/kvm-cpu.c | 105 ++++++++++++++++++++------------------
target/i386/tcg/tcg-cpu.c | 11 ++--
4 files changed, 89 insertions(+), 59 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 48b55ebd0a..edb97ebbbe 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4919,6 +4919,9 @@ static uint64_t
x86_cpu_get_supported_feature_word(FeatureWord w,
return r;
}
+/*
+ * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
+ */
void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
{
PropValue *pv;
@@ -4931,7 +4934,11 @@ void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
}
}
-/* Apply properties for the CPU model version specified in model */
+/*
+ * Apply properties for the CPU model version specified in model.
+ * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
+ */
+
static void x86_cpu_apply_version_props(X86CPU *cpu, X86CPUModel *model)
{
const X86CPUVersionDefinition *vdef;
@@ -4960,7 +4967,9 @@ static void x86_cpu_apply_version_props(X86CPU *cpu,
X86CPUModel *model)
assert(vdef->version == version);
}
-/* Load data from X86CPUDefinition into a X86CPU object
+/*
+ * Load data from X86CPUDefinition into a X86CPU object.
+ * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
*/
static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
{
@@ -5051,6 +5060,12 @@ static void x86_register_cpu_model_type(const char
*name, X86CPUModel *model)
type_register(&ti);
}
+
+/*
+ * register builtin_x86_defs;
+ * "max", "base" and subclasses ("host") are not registered here.
+ * See x86_cpu_register_types for all model registrations.
+ */
static void x86_register_cpudef_types(const X86CPUDefinition *def)
{
X86CPUModel *m;
diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 4ea9e354ea..10f8aba86e 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -150,13 +150,16 @@ void host_cpu_vendor_fms(char *vendor, int *family, int
*model, int *stepping)
void host_cpu_instance_init(X86CPU *cpu)
{
- uint32_t ebx = 0, ecx = 0, edx = 0;
- char vendor[CPUID_VENDOR_SZ + 1];
+ X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
- host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
- x86_cpu_vendor_words2str(vendor, ebx, edx, ecx);
+ if (xcc->model) {
+ uint32_t ebx = 0, ecx = 0, edx = 0;
+ char vendor[CPUID_VENDOR_SZ + 1];
- object_property_set_str(OBJECT(cpu), "vendor", vendor, &error_abort);
+ host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
+ x86_cpu_vendor_words2str(vendor, ebx, edx, ecx);
+ object_property_set_str(OBJECT(cpu), "vendor", vendor, &error_abort);
+ }
}
void host_cpu_max_instance_init(X86CPU *cpu)
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index bbe817764d..d95028018e 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -52,47 +52,6 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
return host_cpu_realizefn(cs, errp);
}
-/*
- * KVM-specific features that are automatically added/removed
- * from all CPU models when KVM is enabled.
- *
- * NOTE: features can be enabled by default only if they were
- * already available in the oldest kernel version supported
- * by the KVM accelerator (see "OS requirements" section at
- * docs/system/target-i386.rst)
- */
-static PropValue kvm_default_props[] = {
- { "kvmclock", "on" },
- { "kvm-nopiodelay", "on" },
- { "kvm-asyncpf", "on" },
- { "kvm-steal-time", "on" },
- { "kvm-pv-eoi", "on" },
- { "kvmclock-stable-bit", "on" },
- { "x2apic", "on" },
- { "kvm-msi-ext-dest-id", "off" },
- { "acpi", "off" },
- { "monitor", "off" },
- { "svm", "off" },
- { NULL, NULL },
-};
-
-void x86_cpu_change_kvm_default(const char *prop, const char *value)
-{
- PropValue *pv;
- for (pv = kvm_default_props; pv->prop; pv++) {
- if (!strcmp(pv->prop, prop)) {
- pv->value = value;
- break;
- }
- }
-
- /*
- * It is valid to call this function only for properties that
- * are already present in the kvm_default_props table.
- */
- assert(pv->prop);
-}
-
static bool lmce_supported(void)
{
uint64_t mce_cap = 0;
@@ -150,21 +109,69 @@ static void kvm_cpu_xsave_init(void)
}
}
+/*
+ * KVM-specific features that are automatically added/removed
+ * from cpudef models when KVM is enabled.
+ * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
+ *
+ * NOTE: features can be enabled by default only if they were
+ * already available in the oldest kernel version supported
+ * by the KVM accelerator (see "OS requirements" section at
+ * docs/system/target-i386.rst)
+ */
+static PropValue kvm_default_props[] = {
+ { "kvmclock", "on" },
+ { "kvm-nopiodelay", "on" },
+ { "kvm-asyncpf", "on" },
+ { "kvm-steal-time", "on" },
+ { "kvm-pv-eoi", "on" },
+ { "kvmclock-stable-bit", "on" },
+ { "x2apic", "on" },
+ { "kvm-msi-ext-dest-id", "off" },
+ { "acpi", "off" },
+ { "monitor", "off" },
+ { "svm", "off" },
+ { NULL, NULL },
+};
+
+/*
+ * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
+ */
+void x86_cpu_change_kvm_default(const char *prop, const char *value)
+{
+ PropValue *pv;
+ for (pv = kvm_default_props; pv->prop; pv++) {
+ if (!strcmp(pv->prop, prop)) {
+ pv->value = value;
+ break;
+ }
+ }
+
+ /*
+ * It is valid to call this function only for properties that
+ * are already present in the kvm_default_props table.
+ */
+ assert(pv->prop);
+}
+
static void kvm_cpu_instance_init(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
+ X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
host_cpu_instance_init(cpu);
- if (!kvm_irqchip_in_kernel()) {
- x86_cpu_change_kvm_default("x2apic", "off");
- } else if (kvm_irqchip_is_split() && kvm_enable_x2apic()) {
- x86_cpu_change_kvm_default("kvm-msi-ext-dest-id", "on");
- }
-
- /* Special cases not set in the X86CPUDefinition structs: */
+ if (xcc->model) {
+ /* only applies to builtin_x86_defs cpus */
+ if (!kvm_irqchip_in_kernel()) {
+ x86_cpu_change_kvm_default("x2apic", "off");
+ } else if (kvm_irqchip_is_split() && kvm_enable_x2apic()) {
+ x86_cpu_change_kvm_default("kvm-msi-ext-dest-id", "on");
+ }
- x86_cpu_apply_props(cpu, kvm_default_props);
+ /* Special cases not set in the X86CPUDefinition structs: */
+ x86_cpu_apply_props(cpu, kvm_default_props);
+ }
if (cpu->max_features) {
kvm_cpu_max_instance_init(cpu);
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index e96ec9bbcc..e86bc93384 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -99,7 +99,8 @@ static void tcg_cpu_xsave_init(void)
}
/*
- * TCG-specific defaults that override all CPU models when using TCG
+ * TCG-specific defaults that override cpudef models when using TCG.
+ * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
*/
static PropValue tcg_default_props[] = {
{ "vme", "off" },
@@ -109,8 +110,12 @@ static PropValue tcg_default_props[] = {
static void tcg_cpu_instance_init(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
- /* Special cases not set in the X86CPUDefinition structs: */
- x86_cpu_apply_props(cpu, tcg_default_props);
+ X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
+
+ if (xcc->model) {
+ /* Special cases not set in the X86CPUDefinition structs: */
+ x86_cpu_apply_props(cpu, tcg_default_props);
+ }
tcg_cpu_xsave_init();
}
--
2.26.2
