* [Qemu-devel] [PULL 01/10] target-i386: add Skylake-Client cpu model
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
@ 2016-06-14 20:58 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 02/10] pc: Add 2.7 machine Eduardo Habkost
` (9 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:58 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini
Introduce Skylake-Client cpu mode which inherits the features from
Broadwell and supports some additional features that are: MPX,
XSAVEC, and XGETBV1.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 895a386..9c5aabc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1239,6 +1239,51 @@ static X86CPUDefinition builtin_x86_defs[] = {
.model_id = "Intel Core Processor (Broadwell)",
},
{
+ .name = "Skylake-Client",
+ .level = 0xd,
+ .vendor = CPUID_VENDOR_INTEL,
+ .family = 6,
+ .model = 94,
+ .stepping = 3,
+ .features[FEAT_1_EDX] =
+ CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+ CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
+ CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
+ CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
+ CPUID_DE | CPUID_FP87,
+ .features[FEAT_1_ECX] =
+ CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+ CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
+ CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
+ CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
+ CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
+ CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
+ .features[FEAT_8000_0001_EDX] =
+ CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
+ CPUID_EXT2_SYSCALL,
+ .features[FEAT_8000_0001_ECX] =
+ CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
+ .features[FEAT_7_0_EBX] =
+ CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
+ CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
+ CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
+ CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
+ CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_MPX,
+ /* Missing: XSAVES (not supported by some Linux versions,
+ * including v4.1 to v4.6).
+ * KVM doesn't yet expose any XSAVES state save component,
+ * and the only one defined in Skylake (processor tracing)
+ * probably will block migration anyway.
+ */
+ .features[FEAT_XSAVE] =
+ CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
+ CPUID_XSAVE_XGETBV1,
+ .features[FEAT_6_EAX] =
+ CPUID_6_EAX_ARAT,
+ .xlevel = 0x80000008,
+ .model_id = "Intel Core Processor (Skylake)",
+ },
+ {
.name = "Opteron_G1",
.level = 5,
.vendor = CPUID_VENDOR_AMD,
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 02/10] pc: Add 2.7 machine
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
2016-06-14 20:58 ` [Qemu-devel] [PULL 01/10] target-i386: add Skylake-Client cpu model Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 03/10] target-i386: Implement CPUID[0xB] (Extended Topology Enumeration) Eduardo Habkost
` (8 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini,
Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/pc_piix.c | 16 +++++++++++++---
hw/i386/pc_q35.c | 13 +++++++++++--
include/hw/i386/pc.h | 4 ++++
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 82c7c0a..53bc968 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -432,13 +432,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
m->default_display = "std";
}
-static void pc_i440fx_2_6_machine_options(MachineClass *m)
+static void pc_i440fx_2_7_machine_options(MachineClass *m)
{
pc_i440fx_machine_options(m);
m->alias = "pc";
m->is_default = 1;
}
+DEFINE_I440FX_MACHINE(v2_7, "pc-i440fx-2.7", NULL,
+ pc_i440fx_2_7_machine_options);
+
+
+static void pc_i440fx_2_6_machine_options(MachineClass *m)
+{
+ pc_i440fx_2_7_machine_options(m);
+ m->is_default = 0;
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_6);
+}
+
DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
pc_i440fx_2_6_machine_options);
@@ -447,8 +459,6 @@ static void pc_i440fx_2_5_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_i440fx_2_6_machine_options(m);
- m->alias = NULL;
- m->is_default = 0;
pcmc->save_tsc_khz = false;
m->legacy_fw_cfg_order = 1;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 31a6a59..e4b541f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -283,12 +283,22 @@ static void pc_q35_machine_options(MachineClass *m)
m->no_floppy = 1;
}
-static void pc_q35_2_6_machine_options(MachineClass *m)
+static void pc_q35_2_7_machine_options(MachineClass *m)
{
pc_q35_machine_options(m);
m->alias = "q35";
}
+DEFINE_Q35_MACHINE(v2_7, "pc-q35-2.7", NULL,
+ pc_q35_2_7_machine_options);
+
+static void pc_q35_2_6_machine_options(MachineClass *m)
+{
+ pc_q35_2_7_machine_options(m);
+ m->alias = NULL;
+ SET_MACHINE_COMPAT(m, PC_COMPAT_2_6);
+}
+
DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL,
pc_q35_2_6_machine_options);
@@ -296,7 +306,6 @@ static void pc_q35_2_5_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_2_6_machine_options(m);
- m->alias = NULL;
pcmc->save_tsc_khz = false;
m->legacy_fw_cfg_order = 1;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9ca2309..ca23609 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -356,7 +356,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
+#define PC_COMPAT_2_6 \
+ HW_COMPAT_2_6
+
#define PC_COMPAT_2_5 \
+ PC_COMPAT_2_6 \
HW_COMPAT_2_5
/* Helper for setting model-id for CPU models that changed model-id
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 03/10] target-i386: Implement CPUID[0xB] (Extended Topology Enumeration)
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
2016-06-14 20:58 ` [Qemu-devel] [PULL 01/10] target-i386: add Skylake-Client cpu model Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 02/10] pc: Add 2.7 machine Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 04/10] target-i386: Remove xlevel & hv-spinlocks option fixups Eduardo Habkost
` (7 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini,
Radim Krčmář
From: Radim Krčmář <rkrcmar@redhat.com>
I looked at a dozen Intel CPU that have this CPUID and all of them
always had Core offset as 1 (a wasted bit when hyperthreading is
disabled) and Package offset at least 4 (wasted bits at <= 4 cores).
QEMU uses more compact IDs and it doesn't make much sense to change it
now. I keep the SMT and Core sub-leaves even if there is just one
thread/core; it makes the code simpler and there should be no harm.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/i386/pc.h | 7 ++++++-
target-i386/cpu.c | 32 ++++++++++++++++++++++++++++++++
target-i386/cpu.h | 8 ++++++++
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ca23609..49566c8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -357,7 +357,12 @@ int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_6 \
- HW_COMPAT_2_6
+ HW_COMPAT_2_6 \
+ {\
+ .driver = TYPE_X86_CPU,\
+ .property = "cpuid-0xb",\
+ .value = "off",\
+ },
#define PC_COMPAT_2_5 \
PC_COMPAT_2_6 \
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9c5aabc..f3f95cd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -41,6 +41,7 @@
#include "sysemu/sysemu.h"
#include "hw/qdev-properties.h"
+#include "hw/i386/topology.h"
#ifndef CONFIG_USER_ONLY
#include "exec/address-spaces.h"
#include "hw/hw.h"
@@ -2492,6 +2493,36 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*edx = 0;
}
break;
+ case 0xB:
+ /* Extended Topology Enumeration Leaf */
+ if (!cpu->enable_cpuid_0xb) {
+ *eax = *ebx = *ecx = *edx = 0;
+ break;
+ }
+
+ *ecx = count & 0xff;
+ *edx = cpu->apic_id;
+
+ switch (count) {
+ case 0:
+ *eax = apicid_core_offset(smp_cores, smp_threads);
+ *ebx = smp_threads;
+ *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
+ break;
+ case 1:
+ *eax = apicid_pkg_offset(smp_cores, smp_threads);
+ *ebx = smp_cores * smp_threads;
+ *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
+ break;
+ default:
+ *eax = 0;
+ *ebx = 0;
+ *ecx |= CPUID_TOPOLOGY_LEVEL_INVALID;
+ }
+
+ assert(!(*eax & ~0x1f));
+ *ebx &= 0xffff; /* The count doesn't need to be reliable. */
+ break;
case 0xD: {
KVMState *s = cs->kvm_state;
uint64_t ena_mask;
@@ -3251,6 +3282,7 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
+ DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
DEFINE_PROP_END_OF_LIST()
};
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 0426459..d9ab884 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -636,6 +636,11 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_MWAIT_IBE (1U << 1) /* Interrupts can exit capability */
#define CPUID_MWAIT_EMX (1U << 0) /* enumeration supported */
+/* CPUID[0xB].ECX level types */
+#define CPUID_TOPOLOGY_LEVEL_INVALID (0U << 8)
+#define CPUID_TOPOLOGY_LEVEL_SMT (1U << 8)
+#define CPUID_TOPOLOGY_LEVEL_CORE (2U << 8)
+
#ifndef HYPERV_SPINLOCK_NEVER_RETRY
#define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF
#endif
@@ -1173,6 +1178,9 @@ struct X86CPU {
*/
bool enable_pmu;
+ /* Compatibility bits for old machine types: */
+ bool enable_cpuid_0xb;
+
/* in order to simplify APIC support, we leave this pointer to the
user */
struct DeviceState *apic_state;
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 04/10] target-i386: Remove xlevel & hv-spinlocks option fixups
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (2 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 03/10] target-i386: Implement CPUID[0xB] (Extended Topology Enumeration) Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 05/10] target-i386: Move features logic that requires CPUState to realize time Eduardo Habkost
` (6 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini
The "fixup will be removed in future versions" warnings are
present since QEMU 1.7.0, at least, so users should have fixed
their scripts and configurations, already.
In the case of libvirt users, libvirt doesn't use the "xlevel"
option, and already rejects HyperV spinlock retry count < 0xFFF.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 36 +-----------------------------------
1 file changed, 1 insertion(+), 35 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f3f95cd..a62d731 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1952,7 +1952,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
FeatureWordArray plus_features = { 0 };
/* Features to be removed */
FeatureWordArray minus_features = { 0 };
- uint32_t numvalue;
CPUX86State *env = &cpu->env;
Error *local_err = NULL;
@@ -1967,23 +1966,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
} else if ((val = strchr(featurestr, '='))) {
*val = 0; val++;
feat2prop(featurestr);
- if (!strcmp(featurestr, "xlevel")) {
- char *err;
- char num[32];
-
- numvalue = strtoul(val, &err, 0);
- if (!*val || *err) {
- error_setg(errp, "bad numerical value %s", val);
- return;
- }
- if (numvalue < 0x80000000) {
- error_report("xlevel value shall always be >= 0x80000000"
- ", fixup will be removed in future versions");
- numvalue += 0x80000000;
- }
- snprintf(num, sizeof(num), "%" PRIu32, numvalue);
- object_property_parse(OBJECT(cpu), num, featurestr, &local_err);
- } else if (!strcmp(featurestr, "tsc-freq")) {
+ if (!strcmp(featurestr, "tsc-freq")) {
int64_t tsc_freq;
char *err;
char num[32];
@@ -1997,23 +1980,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
object_property_parse(OBJECT(cpu), num, "tsc-frequency",
&local_err);
- } else if (!strcmp(featurestr, "hv-spinlocks")) {
- char *err;
- const int min = 0xFFF;
- char num[32];
- numvalue = strtoul(val, &err, 0);
- if (!*val || *err) {
- error_setg(errp, "bad numerical value %s", val);
- return;
- }
- if (numvalue < min) {
- error_report("hv-spinlocks value shall always be >= 0x%x"
- ", fixup will be removed in future versions",
- min);
- numvalue = min;
- }
- snprintf(num, sizeof(num), "%" PRId32, numvalue);
- object_property_parse(OBJECT(cpu), num, featurestr, &local_err);
} else {
object_property_parse(OBJECT(cpu), val, featurestr, &local_err);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 05/10] target-i386: Move features logic that requires CPUState to realize time
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (3 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 04/10] target-i386: Remove xlevel & hv-spinlocks option fixups Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 06/10] target-i386: Remove assert(kvm_enabled()) from host_x86_cpu_initfn() Eduardo Habkost
` (5 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini,
Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
Making x86_cpu_parse_featurestr() a pure convertor
of legacy feature string into global properties, needs
it to be called before a CPU instance is created so
parser shouldn't modify CPUState directly or access
it at all. Hence move current hack that directly pokes
into CPUState, to set/unset +-feats, from parser to
CPU's realize method.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a62d731..647cd30 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1940,6 +1940,14 @@ static inline void feat2prop(char *s)
}
}
+/* Compatibily hack to maintain legacy +-feat semantic,
+ * where +-feat overwrites any feature set by
+ * feat=on|feat even if the later is parsed after +-feat
+ * (i.e. "-x2apic,x2apic=on" will result in x2apic disabled)
+ */
+static FeatureWordArray plus_features = { 0 };
+static FeatureWordArray minus_features = { 0 };
+
/* Parse "+feature,-feature,feature=foo" CPU feature string
*/
static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
@@ -1947,12 +1955,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
{
X86CPU *cpu = X86_CPU(cs);
char *featurestr; /* Single 'key=value" string being parsed */
- FeatureWord w;
- /* Features to be added */
- FeatureWordArray plus_features = { 0 };
- /* Features to be removed */
- FeatureWordArray minus_features = { 0 };
- CPUX86State *env = &cpu->env;
Error *local_err = NULL;
featurestr = features ? strtok(features, ",") : NULL;
@@ -1993,18 +1995,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
}
featurestr = strtok(NULL, ",");
}
-
- if (cpu->host_features) {
- for (w = 0; w < FEATURE_WORDS; w++) {
- env->features[w] =
- x86_cpu_get_supported_feature_word(w, cpu->migratable);
- }
- }
-
- for (w = 0; w < FEATURE_WORDS; w++) {
- env->features[w] |= plus_features[w];
- env->features[w] &= ~minus_features[w];
- }
}
/* Print all cpuid feature names in featureset
@@ -2916,12 +2906,30 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
CPUX86State *env = &cpu->env;
Error *local_err = NULL;
static bool ht_warned;
+ FeatureWord w;
if (cpu->apic_id < 0) {
error_setg(errp, "apic-id property was not initialized properly");
return;
}
+ /*TODO: cpu->host_features incorrectly overwrites features
+ * set using "feat=on|off". Once we fix this, we can convert
+ * plus_features & minus_features to global properties
+ * inside x86_cpu_parse_featurestr() too.
+ */
+ if (cpu->host_features) {
+ for (w = 0; w < FEATURE_WORDS; w++) {
+ env->features[w] =
+ x86_cpu_get_supported_feature_word(w, cpu->migratable);
+ }
+ }
+
+ for (w = 0; w < FEATURE_WORDS; w++) {
+ cpu->env.features[w] |= plus_features[w];
+ cpu->env.features[w] &= ~minus_features[w];
+ }
+
if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) {
env->cpuid_level = 7;
}
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 06/10] target-i386: Remove assert(kvm_enabled()) from host_x86_cpu_initfn()
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (4 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 05/10] target-i386: Move features logic that requires CPUState to realize time Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 07/10] target-i386: Move xcc->kvm_required check to realize time Eduardo Habkost
` (4 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini
The code will be changed to allow creation of the CPU object and
report kvm_required errors only at realizefn, so we need to make
the instance_init function more flexible.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 647cd30..c91902f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1547,16 +1547,17 @@ static void host_x86_cpu_initfn(Object *obj)
CPUX86State *env = &cpu->env;
KVMState *s = kvm_state;
- assert(kvm_enabled());
-
/* We can't fill the features array here because we don't know yet if
* "migratable" is true or false.
*/
cpu->host_features = true;
- env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
- env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
- env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ /* If KVM is disabled, cpu_x86_create() will already report an error */
+ if (kvm_enabled()) {
+ env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+ env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+ env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ }
object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 07/10] target-i386: Move xcc->kvm_required check to realize time
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (5 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 06/10] target-i386: Remove assert(kvm_enabled()) from host_x86_cpu_initfn() Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 08/10] target-i386: Use cpu_generic_init() in cpu_x86_init() Eduardo Habkost
` (3 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini,
Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
It will allow to drop custom cpu_x86_init() and use
cpu_generic_init() instead, reducing cpu_x86_create()
to a simple 3-liner.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c91902f..7db632b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -678,6 +678,14 @@ static ObjectClass *x86_cpu_class_by_name(const char *cpu_model)
return oc;
}
+static char *x86_cpu_class_get_model_name(X86CPUClass *cc)
+{
+ const char *class_name = object_class_get_name(OBJECT_CLASS(cc));
+ assert(g_str_has_suffix(class_name, X86_CPU_TYPE_SUFFIX));
+ return g_strndup(class_name,
+ strlen(class_name) - strlen(X86_CPU_TYPE_SUFFIX));
+}
+
struct X86CPUDefinition {
const char *name;
uint32_t level;
@@ -1552,7 +1560,7 @@ static void host_x86_cpu_initfn(Object *obj)
*/
cpu->host_features = true;
- /* If KVM is disabled, cpu_x86_create() will already report an error */
+ /* If KVM is disabled, x86_cpu_realizefn() will report an error later */
if (kvm_enabled()) {
env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
@@ -2178,7 +2186,6 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
{
X86CPU *cpu = NULL;
- X86CPUClass *xcc;
ObjectClass *oc;
gchar **model_pieces;
char *name, *features;
@@ -2197,12 +2204,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
error_setg(&error, "Unable to find CPU definition: %s", name);
goto out;
}
- xcc = X86_CPU_CLASS(oc);
-
- if (xcc->kvm_required && !kvm_enabled()) {
- error_setg(&error, "CPU model '%s' requires KVM", name);
- goto out;
- }
cpu = X86_CPU(object_new(object_class_get_name(oc)));
@@ -2909,6 +2910,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
static bool ht_warned;
FeatureWord w;
+ if (xcc->kvm_required && !kvm_enabled()) {
+ char *name = x86_cpu_class_get_model_name(xcc);
+ error_setg(&local_err, "CPU model '%s' requires KVM", name);
+ g_free(name);
+ goto out;
+ }
+
if (cpu->apic_id < 0) {
error_setg(errp, "apic-id property was not initialized properly");
return;
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 08/10] target-i386: Use cpu_generic_init() in cpu_x86_init()
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (6 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 07/10] target-i386: Move xcc->kvm_required check to realize time Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 09/10] target-i386: Consolidate calls of object_property_parse() in x86_cpu_parse_featurestr Eduardo Habkost
` (2 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini,
Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
Now cpu_x86_init() does nothing more or less
than duplicating cpu_generic_init() logic.
So simplify it by using cpu_generic_init().
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7db632b..329d85c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2226,25 +2226,7 @@ out:
X86CPU *cpu_x86_init(const char *cpu_model)
{
- Error *error = NULL;
- X86CPU *cpu;
-
- cpu = cpu_x86_create(cpu_model, &error);
- if (error) {
- goto out;
- }
-
- object_property_set_bool(OBJECT(cpu), true, "realized", &error);
-
-out:
- if (error) {
- error_report_err(error);
- if (cpu != NULL) {
- object_unref(OBJECT(cpu));
- cpu = NULL;
- }
- }
- return cpu;
+ return X86_CPU(cpu_generic_init(TYPE_X86_CPU, cpu_model));
}
static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 09/10] target-i386: Consolidate calls of object_property_parse() in x86_cpu_parse_featurestr
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (7 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 08/10] target-i386: Use cpu_generic_init() in cpu_x86_init() Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 20:59 ` [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used Eduardo Habkost
2016-06-14 21:32 ` [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
10 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 74 +++++++++++++++++++++++++++++++++----------------------
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 329d85c..3665fec 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1966,43 +1966,59 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
char *featurestr; /* Single 'key=value" string being parsed */
Error *local_err = NULL;
- featurestr = features ? strtok(features, ",") : NULL;
+ if (!features) {
+ return;
+ }
+
+ for (featurestr = strtok(features, ",");
+ featurestr && !local_err;
+ featurestr = strtok(NULL, ",")) {
+ const char *name;
+ const char *val = NULL;
+ char *eq = NULL;
- while (featurestr) {
- char *val;
+ /* Compatibility syntax: */
if (featurestr[0] == '+') {
add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err);
+ continue;
} else if (featurestr[0] == '-') {
add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err);
- } else if ((val = strchr(featurestr, '='))) {
- *val = 0; val++;
- feat2prop(featurestr);
- if (!strcmp(featurestr, "tsc-freq")) {
- int64_t tsc_freq;
- char *err;
- char num[32];
-
- tsc_freq = qemu_strtosz_suffix_unit(val, &err,
- QEMU_STRTOSZ_DEFSUFFIX_B, 1000);
- if (tsc_freq < 0 || *err) {
- error_setg(errp, "bad numerical value %s", val);
- return;
- }
- snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
- object_property_parse(OBJECT(cpu), num, "tsc-frequency",
- &local_err);
- } else {
- object_property_parse(OBJECT(cpu), val, featurestr, &local_err);
- }
+ continue;
+ }
+
+ eq = strchr(featurestr, '=');
+ if (eq) {
+ *eq++ = 0;
+ val = eq;
} else {
- feat2prop(featurestr);
- object_property_parse(OBJECT(cpu), "on", featurestr, &local_err);
+ val = "on";
}
- if (local_err) {
- error_propagate(errp, local_err);
- return;
+
+ feat2prop(featurestr);
+ name = featurestr;
+
+ /* Special case: */
+ if (!strcmp(name, "tsc-freq")) {
+ int64_t tsc_freq;
+ char *err;
+ char num[32];
+
+ tsc_freq = qemu_strtosz_suffix_unit(val, &err,
+ QEMU_STRTOSZ_DEFSUFFIX_B, 1000);
+ if (tsc_freq < 0 || *err) {
+ error_setg(errp, "bad numerical value %s", val);
+ return;
+ }
+ snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
+ val = num;
+ name = "tsc-frequency";
}
- featurestr = strtok(NULL, ",");
+
+ object_property_parse(OBJECT(cpu), val, name, &local_err);
+ }
+
+ if (local_err) {
+ error_propagate(errp, local_err);
}
}
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (8 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 09/10] target-i386: Consolidate calls of object_property_parse() in x86_cpu_parse_featurestr Eduardo Habkost
@ 2016-06-14 20:59 ` Eduardo Habkost
2016-06-14 21:16 ` Paolo Bonzini
2016-06-14 21:32 ` [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
10 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 20:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini,
Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[ehabkost: Changed to use error_report()]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3665fec..baa3783 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1980,9 +1980,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
/* Compatibility syntax: */
if (featurestr[0] == '+') {
add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err);
+ error_report(
+ "'+%s' is obsolete and will be removed in future, use '%s=on'",
+ featurestr + 1, featurestr + 1);
continue;
} else if (featurestr[0] == '-') {
add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err);
+ error_report(
+ "'-%s' is obsolete and will be removed in future, use '%s=off'",
+ featurestr + 1, featurestr + 1);
continue;
}
--
2.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
2016-06-14 20:59 ` [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used Eduardo Habkost
@ 2016-06-14 21:16 ` Paolo Bonzini
2016-06-14 21:31 ` Eduardo Habkost
0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2016-06-14 21:16 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Andreas Färber, qemu-devel, Richard Henderson,
Igor Mammedov
----- Original Message -----
> From: "Eduardo Habkost" <ehabkost@redhat.com>
> To: "Peter Maydell" <peter.maydell@linaro.org>
> Cc: "Andreas Färber" <afaerber@suse.de>, qemu-devel@nongnu.org, "Richard Henderson" <rth@twiddle.net>, "Paolo
> Bonzini" <pbonzini@redhat.com>, "Igor Mammedov" <imammedo@redhat.com>
> Sent: Tuesday, June 14, 2016 10:59:08 PM
> Subject: [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
>
> From: Igor Mammedov <imammedo@redhat.com>
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> [ehabkost: Changed to use error_report()]
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3665fec..baa3783 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1980,9 +1980,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> char *features,
> /* Compatibility syntax: */
> if (featurestr[0] == '+') {
> add_flagname_to_bitmaps(featurestr + 1, plus_features,
> &local_err);
> + error_report(
> + "'+%s' is obsolete and will be removed in future, use
> '%s=on'",
> + featurestr + 1, featurestr + 1);
> continue;
> } else if (featurestr[0] == '-') {
> add_flagname_to_bitmaps(featurestr + 1, minus_features,
> &local_err);
> + error_report(
> + "'-%s' is obsolete and will be removed in future, use
> '%s=off'",
> + featurestr + 1, featurestr + 1);
> continue;
> }
I still disagree with this change.
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
2016-06-14 21:16 ` Paolo Bonzini
@ 2016-06-14 21:31 ` Eduardo Habkost
2016-06-14 21:38 ` Paolo Bonzini
0 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 21:31 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Andreas Färber, qemu-devel, Richard Henderson,
Igor Mammedov
On Tue, Jun 14, 2016 at 05:16:40PM -0400, Paolo Bonzini wrote:
> ----- Original Message -----
> > From: "Eduardo Habkost" <ehabkost@redhat.com>
> > To: "Peter Maydell" <peter.maydell@linaro.org>
> > Cc: "Andreas Färber" <afaerber@suse.de>, qemu-devel@nongnu.org, "Richard Henderson" <rth@twiddle.net>, "Paolo
> > Bonzini" <pbonzini@redhat.com>, "Igor Mammedov" <imammedo@redhat.com>
> > Sent: Tuesday, June 14, 2016 10:59:08 PM
> > Subject: [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
> >
> > From: Igor Mammedov <imammedo@redhat.com>
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > [ehabkost: Changed to use error_report()]
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > target-i386/cpu.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 3665fec..baa3783 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1980,9 +1980,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> > char *features,
> > /* Compatibility syntax: */
> > if (featurestr[0] == '+') {
> > add_flagname_to_bitmaps(featurestr + 1, plus_features,
> > &local_err);
> > + error_report(
> > + "'+%s' is obsolete and will be removed in future, use
> > '%s=on'",
> > + featurestr + 1, featurestr + 1);
> > continue;
> > } else if (featurestr[0] == '-') {
> > add_flagname_to_bitmaps(featurestr + 1, minus_features,
> > &local_err);
> > + error_report(
> > + "'-%s' is obsolete and will be removed in future, use
> > '%s=off'",
> > + featurestr + 1, featurestr + 1);
> > continue;
> > }
>
> I still disagree with this change.
I've just removed the patch from the x86-pull-request tag, while
we sort this out.
Do you suggest supporting the "[+-]feature" syntax forever? If
that's really what you prefer, I won't complain too loudly. It's
only a few extra lines of code, after all.
But if you have something else in mind, please clarify what you
suggest.
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
2016-06-14 21:31 ` Eduardo Habkost
@ 2016-06-14 21:38 ` Paolo Bonzini
2016-06-14 21:46 ` Eduardo Habkost
0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2016-06-14 21:38 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Andreas Färber, qemu-devel, Richard Henderson,
Igor Mammedov
----- Original Message -----
> From: "Eduardo Habkost" <ehabkost@redhat.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Peter Maydell" <peter.maydell@linaro.org>, "Andreas Färber" <afaerber@suse.de>, qemu-devel@nongnu.org, "Richard
> Henderson" <rth@twiddle.net>, "Igor Mammedov" <imammedo@redhat.com>
> Sent: Tuesday, June 14, 2016 11:31:03 PM
> Subject: Re: [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
>
> On Tue, Jun 14, 2016 at 05:16:40PM -0400, Paolo Bonzini wrote:
> > ----- Original Message -----
> > > From: "Eduardo Habkost" <ehabkost@redhat.com>
> > > To: "Peter Maydell" <peter.maydell@linaro.org>
> > > Cc: "Andreas Färber" <afaerber@suse.de>, qemu-devel@nongnu.org, "Richard
> > > Henderson" <rth@twiddle.net>, "Paolo
> > > Bonzini" <pbonzini@redhat.com>, "Igor Mammedov" <imammedo@redhat.com>
> > > Sent: Tuesday, June 14, 2016 10:59:08 PM
> > > Subject: [PULL 10/10] target-i386: Print obsolete warnings if +-features
> > > are used
> > >
> > > From: Igor Mammedov <imammedo@redhat.com>
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > [ehabkost: Changed to use error_report()]
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > target-i386/cpu.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index 3665fec..baa3783 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -1980,9 +1980,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> > > char *features,
> > > /* Compatibility syntax: */
> > > if (featurestr[0] == '+') {
> > > add_flagname_to_bitmaps(featurestr + 1, plus_features,
> > > &local_err);
> > > + error_report(
> > > + "'+%s' is obsolete and will be removed in future, use
> > > '%s=on'",
> > > + featurestr + 1, featurestr + 1);
> > > continue;
> > > } else if (featurestr[0] == '-') {
> > > add_flagname_to_bitmaps(featurestr + 1, minus_features,
> > > &local_err);
> > > + error_report(
> > > + "'-%s' is obsolete and will be removed in future, use
> > > '%s=off'",
> > > + featurestr + 1, featurestr + 1);
> > > continue;
> > > }
> >
> > I still disagree with this change.
>
> I've just removed the patch from the x86-pull-request tag, while
> we sort this out.
>
> Do you suggest supporting the "[+-]feature" syntax forever?
I suggest supporting it, but removing the awful interaction with "feature=on/off"
as soon as possible. This shouldn't block this pull request, of course.
I just believe it's not practical to remove the feature. For example
kvm-unit-tests can be used with new kernel and old QEMU, so I don't think
it will move away from [+-]feature very soon.
Regarding libvirt, is "feature=on/off" introspectable? That would also be
a problem for libvirt to support both old and new QEMU.
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
2016-06-14 21:38 ` Paolo Bonzini
@ 2016-06-14 21:46 ` Eduardo Habkost
0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 21:46 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Andreas Färber, qemu-devel, Richard Henderson,
Igor Mammedov
On Tue, Jun 14, 2016 at 05:38:42PM -0400, Paolo Bonzini wrote:
>
>
> ----- Original Message -----
> > From: "Eduardo Habkost" <ehabkost@redhat.com>
> > To: "Paolo Bonzini" <pbonzini@redhat.com>
> > Cc: "Peter Maydell" <peter.maydell@linaro.org>, "Andreas Färber" <afaerber@suse.de>, qemu-devel@nongnu.org, "Richard
> > Henderson" <rth@twiddle.net>, "Igor Mammedov" <imammedo@redhat.com>
> > Sent: Tuesday, June 14, 2016 11:31:03 PM
> > Subject: Re: [PULL 10/10] target-i386: Print obsolete warnings if +-features are used
> >
> > On Tue, Jun 14, 2016 at 05:16:40PM -0400, Paolo Bonzini wrote:
> > > ----- Original Message -----
> > > > From: "Eduardo Habkost" <ehabkost@redhat.com>
> > > > To: "Peter Maydell" <peter.maydell@linaro.org>
> > > > Cc: "Andreas Färber" <afaerber@suse.de>, qemu-devel@nongnu.org, "Richard
> > > > Henderson" <rth@twiddle.net>, "Paolo
> > > > Bonzini" <pbonzini@redhat.com>, "Igor Mammedov" <imammedo@redhat.com>
> > > > Sent: Tuesday, June 14, 2016 10:59:08 PM
> > > > Subject: [PULL 10/10] target-i386: Print obsolete warnings if +-features
> > > > are used
> > > >
> > > > From: Igor Mammedov <imammedo@redhat.com>
> > > >
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > [ehabkost: Changed to use error_report()]
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > ---
> > > > target-i386/cpu.c | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > > index 3665fec..baa3783 100644
> > > > --- a/target-i386/cpu.c
> > > > +++ b/target-i386/cpu.c
> > > > @@ -1980,9 +1980,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> > > > char *features,
> > > > /* Compatibility syntax: */
> > > > if (featurestr[0] == '+') {
> > > > add_flagname_to_bitmaps(featurestr + 1, plus_features,
> > > > &local_err);
> > > > + error_report(
> > > > + "'+%s' is obsolete and will be removed in future, use
> > > > '%s=on'",
> > > > + featurestr + 1, featurestr + 1);
> > > > continue;
> > > > } else if (featurestr[0] == '-') {
> > > > add_flagname_to_bitmaps(featurestr + 1, minus_features,
> > > > &local_err);
> > > > + error_report(
> > > > + "'-%s' is obsolete and will be removed in future, use
> > > > '%s=off'",
> > > > + featurestr + 1, featurestr + 1);
> > > > continue;
> > > > }
> > >
> > > I still disagree with this change.
> >
> > I've just removed the patch from the x86-pull-request tag, while
> > we sort this out.
> >
> > Do you suggest supporting the "[+-]feature" syntax forever?
>
> I suggest supporting it, but removing the awful interaction with "feature=on/off"
> as soon as possible. This shouldn't block this pull request, of course.
I plan to fix the awful ordering semantics. First with a warning
for 1 or 2 releases (only when the weird semantics is really
triggered), then +feature/-feature could be directly translated
to feature=on/feature=off.
>
> I just believe it's not practical to remove the feature. For example
> kvm-unit-tests can be used with new kernel and old QEMU, so I don't think
> it will move away from [+-]feature very soon.
>
> Regarding libvirt, is "feature=on/off" introspectable? That would also be
> a problem for libvirt to support both old and new QEMU.
Good point. Removing the feature would require dozens of extra
compatibility code to libvirt and kvm-unit-tests just to save 6
lines of code in QEMU. You convinced me.
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14
2016-06-14 20:58 [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
` (9 preceding siblings ...)
2016-06-14 20:59 ` [Qemu-devel] [PULL 10/10] target-i386: Print obsolete warnings if +-features are used Eduardo Habkost
@ 2016-06-14 21:32 ` Eduardo Habkost
2016-06-16 9:53 ` Peter Maydell
10 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-06-14 21:32 UTC (permalink / raw)
To: Peter Maydell
Cc: Andreas Färber, qemu-devel, Richard Henderson, Paolo Bonzini
x86-pull-request tag was updated (patch 10/10 was removed). See
updated pull request info below.
The following changes since commit 49237b856ae58ee7955be0b959c504c51b014f20:
Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20160614-tag' into staging (2016-06-14 16:32:32 +0100)
are available in the git repository at:
git://github.com/ehabkost/qemu.git tags/x86-pull-request
for you to fetch changes up to f6750e959a397dea988efd4e488e1ff813011065:
target-i386: Consolidate calls of object_property_parse() in x86_cpu_parse_featurestr (2016-06-14 16:17:09 -0300)
----------------------------------------------------------------
X86 queue, 2016-06-14 (v2)
----------------------------------------------------------------
Eduardo Habkost (4):
target-i386: add Skylake-Client cpu model
target-i386: Remove xlevel & hv-spinlocks option fixups
target-i386: Remove assert(kvm_enabled()) from host_x86_cpu_initfn()
target-i386: Consolidate calls of object_property_parse() in
x86_cpu_parse_featurestr
Igor Mammedov (4):
pc: Add 2.7 machine
target-i386: Move features logic that requires CPUState to realize
time
target-i386: Move xcc->kvm_required check to realize time
target-i386: Use cpu_generic_init() in cpu_x86_init()
Radim Krčmář (1):
target-i386: Implement CPUID[0xB] (Extended Topology Enumeration)
hw/i386/pc_piix.c | 16 ++-
hw/i386/pc_q35.c | 13 ++-
include/hw/i386/pc.h | 9 ++
target-i386/cpu.c | 276 +++++++++++++++++++++++++++++++--------------------
target-i386/cpu.h | 8 ++
5 files changed, 208 insertions(+), 114 deletions(-)
--
2.5.5
On Tue, Jun 14, 2016 at 05:58:58PM -0300, Eduardo Habkost wrote:
> The following changes since commit 49237b856ae58ee7955be0b959c504c51b014f20:
>
> Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20160614-tag' into staging (2016-06-14 16:32:32 +0100)
>
> are available in the git repository at:
>
> git://github.com/ehabkost/qemu.git tags/x86-pull-request
>
> for you to fetch changes up to bf68389515c16dbc8cc1f082e724c574d57f7c4d:
>
> target-i386: Print obsolete warnings if +-features are used (2016-06-14 16:17:09 -0300)
>
> ----------------------------------------------------------------
> X86 queue, 2016-06-14
>
> ----------------------------------------------------------------
>
> Eduardo Habkost (4):
> target-i386: add Skylake-Client cpu model
> target-i386: Remove xlevel & hv-spinlocks option fixups
> target-i386: Remove assert(kvm_enabled()) from host_x86_cpu_initfn()
> target-i386: Consolidate calls of object_property_parse() in
> x86_cpu_parse_featurestr
>
> Igor Mammedov (5):
> pc: Add 2.7 machine
> target-i386: Move features logic that requires CPUState to realize
> time
> target-i386: Move xcc->kvm_required check to realize time
> target-i386: Use cpu_generic_init() in cpu_x86_init()
> target-i386: Print obsolete warnings if +-features are used
>
> Radim Krčmář (1):
> target-i386: Implement CPUID[0xB] (Extended Topology Enumeration)
>
> hw/i386/pc_piix.c | 16 ++-
> hw/i386/pc_q35.c | 13 ++-
> include/hw/i386/pc.h | 9 ++
> target-i386/cpu.c | 282 +++++++++++++++++++++++++++++++--------------------
> target-i386/cpu.h | 8 ++
> 5 files changed, 214 insertions(+), 114 deletions(-)
>
> --
> 2.5.5
>
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14
2016-06-14 21:32 ` [Qemu-devel] [PULL 00/10] X86 queue, 2016-06-14 Eduardo Habkost
@ 2016-06-16 9:53 ` Peter Maydell
0 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-06-16 9:53 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Andreas Färber, QEMU Developers, Richard Henderson,
Paolo Bonzini
On 14 June 2016 at 22:32, Eduardo Habkost <ehabkost@redhat.com> wrote:
> x86-pull-request tag was updated (patch 10/10 was removed). See
> updated pull request info below.
>
> The following changes since commit 49237b856ae58ee7955be0b959c504c51b014f20:
>
> Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20160614-tag' into staging (2016-06-14 16:32:32 +0100)
>
> are available in the git repository at:
>
> git://github.com/ehabkost/qemu.git tags/x86-pull-request
>
> for you to fetch changes up to f6750e959a397dea988efd4e488e1ff813011065:
>
> target-i386: Consolidate calls of object_property_parse() in x86_cpu_parse_featurestr (2016-06-14 16:17:09 -0300)
>
> ----------------------------------------------------------------
> X86 queue, 2016-06-14 (v2)
>
> ----------------------------------------------------------------
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 17+ messages in thread