All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-25 20:45 ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

Changes v1 -> v2:
 * Commit message and comment changes.
 * Update compat code to change pc-*-2.1, not pc-*-2.0.
 * Added patch to disable SVM by default in KVM mode.

Most of the bits that make "enforce" breaks were introduced in 2010 by commit
8560efed6a72a816c0115f41ddb9d79f7ce63f28. The intention behind that commit made
sense, the only problem is that we can't guarantee guest ABI stability across
hosts if we simply rely on trimming of CPU features based on host capabilities.

So, this series remove CPUID bits from the CPU model definitions so they become
defaults that: 1) won't unexpectly stop working when we start using the
"enforce" flag; 2) won't silently break the guest ABI when TCG or KVM start
supporting new features.

There's only one non-trivial case left: the qemu32/qemu64 models. The problem
with them is that we have conflicting expectations about it, from different
users:

TCG users expect the default CPU model to contain most TCG-supported features
(and it makes sense). See, for example, commit
f1e00a9cf326acc1f2386a72525af8859852e1df.

KVM users expect the default CPU model to be a conservative choice which will
work on most host CPUs (and will only contain features that are supported by
KVM).

We could solve the qemu32/qemu64 issue by having different defaults for TCG and
KVM. But we have existing management code (libvirt) that already expects qemu32
or qemu64 to be the default, and changing the default would break that code. I
will send an RFC to address that later.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org

Eduardo Habkost (6):
  pc: Create pc_compat_2_1() functions
  target-i386: Rename KVM auto-feature-enable compat function
  target-i386: Disable CPUID_ACPI by default on KVM mode
  target-i386: Remove unsupported bits from all CPU models
  target-i386: Don't enable nested VMX by default
  target-i386: Disable SVM by default in KVM mode

 hw/i386/pc_piix.c | 22 ++++++++++++++++++----
 hw/i386/pc_q35.c  | 18 ++++++++++++++++--
 target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
 target-i386/cpu.h |  3 ++-
 4 files changed, 64 insertions(+), 21 deletions(-)

-- 
1.9.3


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-25 20:45 ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

Changes v1 -> v2:
 * Commit message and comment changes.
 * Update compat code to change pc-*-2.1, not pc-*-2.0.
 * Added patch to disable SVM by default in KVM mode.

Most of the bits that make "enforce" breaks were introduced in 2010 by commit
8560efed6a72a816c0115f41ddb9d79f7ce63f28. The intention behind that commit made
sense, the only problem is that we can't guarantee guest ABI stability across
hosts if we simply rely on trimming of CPU features based on host capabilities.

So, this series remove CPUID bits from the CPU model definitions so they become
defaults that: 1) won't unexpectly stop working when we start using the
"enforce" flag; 2) won't silently break the guest ABI when TCG or KVM start
supporting new features.

There's only one non-trivial case left: the qemu32/qemu64 models. The problem
with them is that we have conflicting expectations about it, from different
users:

TCG users expect the default CPU model to contain most TCG-supported features
(and it makes sense). See, for example, commit
f1e00a9cf326acc1f2386a72525af8859852e1df.

KVM users expect the default CPU model to be a conservative choice which will
work on most host CPUs (and will only contain features that are supported by
KVM).

We could solve the qemu32/qemu64 issue by having different defaults for TCG and
KVM. But we have existing management code (libvirt) that already expects qemu32
or qemu64 to be the default, and changing the default would break that code. I
will send an RFC to address that later.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org

Eduardo Habkost (6):
  pc: Create pc_compat_2_1() functions
  target-i386: Rename KVM auto-feature-enable compat function
  target-i386: Disable CPUID_ACPI by default on KVM mode
  target-i386: Remove unsupported bits from all CPU models
  target-i386: Don't enable nested VMX by default
  target-i386: Disable SVM by default in KVM mode

 hw/i386/pc_piix.c | 22 ++++++++++++++++++----
 hw/i386/pc_q35.c  | 18 ++++++++++++++++--
 target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
 target-i386/cpu.h |  3 ++-
 4 files changed, 64 insertions(+), 21 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 1/6] pc: Create pc_compat_2_1() functions
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-25 20:45   ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

We will need new compat code for the 2.1 machine-types.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 13 ++++++++++++-
 hw/i386/pc_q35.c  | 13 ++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 47ac1b5..5b7f9ba 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -302,8 +302,13 @@ static void pc_init_pci(MachineState *machine)
     pc_init1(machine, 1, 1);
 }
 
+static void pc_compat_2_1(MachineState *machine)
+{
+}
+
 static void pc_compat_2_0(MachineState *machine)
 {
+    pc_compat_2_1(machine);
     /* This value depends on the actual DSDT and SSDT compiled into
      * the source QEMU; unfortunately it depends on the binary and
      * not on the machine type, so we cannot make pc-i440fx-1.7 work on
@@ -367,6 +372,12 @@ static void pc_compat_1_2(MachineState *machine)
     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
 }
 
+static void pc_init_pci_2_1(MachineState *machine)
+{
+    pc_compat_2_1(machine);
+    pc_init_pci(machine);
+}
+
 static void pc_init_pci_2_0(MachineState *machine)
 {
     pc_compat_2_0(machine);
@@ -470,7 +481,7 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
 static QEMUMachine pc_i440fx_machine_v2_1 = {
     PC_I440FX_2_1_MACHINE_OPTIONS,
     .name = "pc-i440fx-2.1",
-    .init = pc_init_pci,
+    .init = pc_init_pci_2_1,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_2_1,
         { /* end of list */ }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4b5a274..602daa8 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -276,8 +276,13 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
+static void pc_compat_2_1(MachineState *machine)
+{
+}
+
 static void pc_compat_2_0(MachineState *machine)
 {
+    pc_compat_2_1(machine);
     smbios_legacy_mode = true;
     has_reserved_memory = false;
 }
@@ -310,6 +315,12 @@ static void pc_compat_1_4(MachineState *machine)
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
 
+static void pc_q35_init_2_1(MachineState *machine)
+{
+    pc_compat_2_1(machine);
+    pc_q35_init(machine);
+}
+
 static void pc_q35_init_2_0(MachineState *machine)
 {
     pc_compat_2_0(machine);
@@ -361,7 +372,7 @@ static QEMUMachine pc_q35_machine_v2_2 = {
 static QEMUMachine pc_q35_machine_v2_1 = {
     PC_Q35_2_1_MACHINE_OPTIONS,
     .name = "pc-q35-2.1",
-    .init = pc_q35_init,
+    .init = pc_q35_init_2_1,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_2_1,
         { /* end of list */ }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 1/6] pc: Create pc_compat_2_1() functions
@ 2014-08-25 20:45   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

We will need new compat code for the 2.1 machine-types.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 13 ++++++++++++-
 hw/i386/pc_q35.c  | 13 ++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 47ac1b5..5b7f9ba 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -302,8 +302,13 @@ static void pc_init_pci(MachineState *machine)
     pc_init1(machine, 1, 1);
 }
 
+static void pc_compat_2_1(MachineState *machine)
+{
+}
+
 static void pc_compat_2_0(MachineState *machine)
 {
+    pc_compat_2_1(machine);
     /* This value depends on the actual DSDT and SSDT compiled into
      * the source QEMU; unfortunately it depends on the binary and
      * not on the machine type, so we cannot make pc-i440fx-1.7 work on
@@ -367,6 +372,12 @@ static void pc_compat_1_2(MachineState *machine)
     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
 }
 
+static void pc_init_pci_2_1(MachineState *machine)
+{
+    pc_compat_2_1(machine);
+    pc_init_pci(machine);
+}
+
 static void pc_init_pci_2_0(MachineState *machine)
 {
     pc_compat_2_0(machine);
@@ -470,7 +481,7 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
 static QEMUMachine pc_i440fx_machine_v2_1 = {
     PC_I440FX_2_1_MACHINE_OPTIONS,
     .name = "pc-i440fx-2.1",
-    .init = pc_init_pci,
+    .init = pc_init_pci_2_1,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_2_1,
         { /* end of list */ }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4b5a274..602daa8 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -276,8 +276,13 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
+static void pc_compat_2_1(MachineState *machine)
+{
+}
+
 static void pc_compat_2_0(MachineState *machine)
 {
+    pc_compat_2_1(machine);
     smbios_legacy_mode = true;
     has_reserved_memory = false;
 }
@@ -310,6 +315,12 @@ static void pc_compat_1_4(MachineState *machine)
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
 
+static void pc_q35_init_2_1(MachineState *machine)
+{
+    pc_compat_2_1(machine);
+    pc_q35_init(machine);
+}
+
 static void pc_q35_init_2_0(MachineState *machine)
 {
     pc_compat_2_0(machine);
@@ -361,7 +372,7 @@ static QEMUMachine pc_q35_machine_v2_2 = {
 static QEMUMachine pc_q35_machine_v2_1 = {
     PC_Q35_2_1_MACHINE_OPTIONS,
     .name = "pc-q35-2.1",
-    .init = pc_q35_init,
+    .init = pc_q35_init_2_1,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_2_1,
         { /* end of list */ }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 2/6] target-i386: Rename KVM auto-feature-enable compat function
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-25 20:45   ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

The x86_cpu_compat_disable_kvm_features() name was a bit confusing, as
it won't forcibly disable the feature for all CPU models (i.e. add it to
kvm_default_unset_features), but it will instead turn off the KVM
auto-enabling of the feature (i.e. remove it from kvm_default_features),
meaning the feature may still be enabled by default in some CPU models).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 6 +++---
 hw/i386/pc_q35.c  | 2 +-
 target-i386/cpu.c | 2 +-
 target-i386/cpu.h | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5b7f9ba..6ee8dfa 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -337,7 +337,7 @@ static void pc_compat_1_7(MachineState *machine)
     gigabyte_align = false;
     option_rom_has_mr = true;
     legacy_acpi_table_size = 6414;
-    x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
 }
 
 static void pc_compat_1_6(MachineState *machine)
@@ -369,7 +369,7 @@ static void pc_compat_1_3(MachineState *machine)
 static void pc_compat_1_2(MachineState *machine)
 {
     pc_compat_1_3(machine);
-    x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, KVM_FEATURE_PV_EOI);
 }
 
 static void pc_init_pci_2_1(MachineState *machine)
@@ -440,7 +440,7 @@ static void pc_init_isa(MachineState *machine)
     if (!machine->cpu_model) {
         machine->cpu_model = "486";
     }
-    x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
     pc_init1(machine, 0, 1);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 602daa8..55fc62f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -293,7 +293,7 @@ static void pc_compat_1_7(MachineState *machine)
     smbios_defaults = false;
     gigabyte_align = false;
     option_rom_has_mr = true;
-    x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
 }
 
 static void pc_compat_1_6(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 217500c..0396410 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -464,7 +464,7 @@ static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
     [FEAT_1_ECX] = CPUID_EXT_MONITOR,
 };
 
-void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features)
+void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
 {
     kvm_default_features[w] &= ~features;
 }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e634d83..346eac1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1300,7 +1300,7 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
 void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
 
-void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features);
+void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 2/6] target-i386: Rename KVM auto-feature-enable compat function
@ 2014-08-25 20:45   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

The x86_cpu_compat_disable_kvm_features() name was a bit confusing, as
it won't forcibly disable the feature for all CPU models (i.e. add it to
kvm_default_unset_features), but it will instead turn off the KVM
auto-enabling of the feature (i.e. remove it from kvm_default_features),
meaning the feature may still be enabled by default in some CPU models).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 6 +++---
 hw/i386/pc_q35.c  | 2 +-
 target-i386/cpu.c | 2 +-
 target-i386/cpu.h | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5b7f9ba..6ee8dfa 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -337,7 +337,7 @@ static void pc_compat_1_7(MachineState *machine)
     gigabyte_align = false;
     option_rom_has_mr = true;
     legacy_acpi_table_size = 6414;
-    x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
 }
 
 static void pc_compat_1_6(MachineState *machine)
@@ -369,7 +369,7 @@ static void pc_compat_1_3(MachineState *machine)
 static void pc_compat_1_2(MachineState *machine)
 {
     pc_compat_1_3(machine);
-    x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, KVM_FEATURE_PV_EOI);
 }
 
 static void pc_init_pci_2_1(MachineState *machine)
@@ -440,7 +440,7 @@ static void pc_init_isa(MachineState *machine)
     if (!machine->cpu_model) {
         machine->cpu_model = "486";
     }
-    x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
     pc_init1(machine, 0, 1);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 602daa8..55fc62f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -293,7 +293,7 @@ static void pc_compat_1_7(MachineState *machine)
     smbios_defaults = false;
     gigabyte_align = false;
     option_rom_has_mr = true;
-    x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
+    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
 }
 
 static void pc_compat_1_6(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 217500c..0396410 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -464,7 +464,7 @@ static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
     [FEAT_1_ECX] = CPUID_EXT_MONITOR,
 };
 
-void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features)
+void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
 {
     kvm_default_features[w] &= ~features;
 }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e634d83..346eac1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1300,7 +1300,7 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
 void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
 
-void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features);
+void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 3/6] target-i386: Disable CPUID_ACPI by default on KVM mode
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-25 20:45   ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

KVM never supported the CPUID_ACPI flag, so it doesn't make sense to
have it enabled by default when KVM is enabled.

The motivation here is exactly the same we had for the MONITOR flag
(disabled by commit 136a7e9a85d7047461f8153f7d12c514a3d68f69).

And like on the MONITOR flag case, we don't need machine-type compat code
because it is currently impossible to run a KVM VM with the ACPI flag set.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0396410..b7fc6e0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -461,6 +461,7 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = {
 /* Features that are not added by default to any CPU model when KVM is enabled.
  */
 static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
+    [FEAT_1_EDX] = CPUID_ACPI,
     [FEAT_1_ECX] = CPUID_EXT_MONITOR,
 };
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 3/6] target-i386: Disable CPUID_ACPI by default on KVM mode
@ 2014-08-25 20:45   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

KVM never supported the CPUID_ACPI flag, so it doesn't make sense to
have it enabled by default when KVM is enabled.

The motivation here is exactly the same we had for the MONITOR flag
(disabled by commit 136a7e9a85d7047461f8153f7d12c514a3d68f69).

And like on the MONITOR flag case, we don't need machine-type compat code
because it is currently impossible to run a KVM VM with the ACPI flag set.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0396410..b7fc6e0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -461,6 +461,7 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = {
 /* Features that are not added by default to any CPU model when KVM is enabled.
  */
 static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
+    [FEAT_1_EDX] = CPUID_ACPI,
     [FEAT_1_ECX] = CPUID_EXT_MONITOR,
 };
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 4/6] target-i386: Remove unsupported bits from all CPU models
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-25 20:45   ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

The following CPU features were never supported by neither TCG or KVM,
so they are useless on the CPU model definitions, today:

 * CPUID_DTS (DS)
 * CPUID_HT
 * CPUID_TM
 * CPUID_PBE
 * CPUID_EXT_DTES64
 * CPUID_EXT_DSCPL
 * CPUID_EXT_EST
 * CPUID_EXT_TM2
 * CPUID_EXT_XTPR
 * CPUID_EXT_PDCM
 * CPUID_SVM_LBRV

As using "enforce" mode is the only way to ensure guest ABI doesn't
change when moving to a different host, we should make "enforce" mode
the default or at least encourage management software to always use it.

In turn, to make "enforce" usable, we need CPU models that work without
always requiring some features to be explicitly disabled. This patch
removes the above features from all CPU model definitions.

We won't need any machine-type compat code for those changes, because it
is impossible to have existing VMs with those features enabled.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
Changes v1 -> v2:
* Trivial typo fix in comment
---
 target-i386/cpu.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b7fc6e0..6f26169 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -680,10 +680,11 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 16,
         .model = 2,
         .stepping = 3,
+        /* Missing: CPUID_HT */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
-            CPUID_PSE36 | CPUID_VME | CPUID_HT,
+            CPUID_PSE36 | CPUID_VME,
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
             CPUID_EXT_POPCNT,
@@ -699,8 +700,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
+        /* Missing: CPUID_SVM_LBRV */
         .features[FEAT_SVM] =
-            CPUID_SVM_NPT | CPUID_SVM_LBRV,
+            CPUID_SVM_NPT,
         .xlevel = 0x8000001A,
         .model_id = "AMD Phenom(tm) 9550 Quad-Core Processor"
     },
@@ -711,15 +713,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 6,
         .model = 15,
         .stepping = 11,
+        /* Missing: CPUID_DTS, CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
-            CPUID_PSE36 | CPUID_VME | CPUID_DTS | CPUID_ACPI | CPUID_SS |
-            CPUID_HT | CPUID_TM | CPUID_PBE,
+            CPUID_PSE36 | CPUID_VME | CPUID_ACPI | CPUID_SS,
+        /* Missing: CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_EST,
+         * CPUID_EXT_TM2, CPUID_EXT_XTPR, CPUID_EXT_PDCM */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_DTES64 | CPUID_EXT_DSCPL | CPUID_EXT_VMX | CPUID_EXT_EST |
-            CPUID_EXT_TM2 | CPUID_EXT_CX16 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
+            CPUID_EXT_VMX | CPUID_EXT_CX16,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
@@ -794,13 +797,15 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 6,
         .model = 14,
         .stepping = 8,
+        /* Missing: CPUID_DTS, CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES | CPUID_VME |
-            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_DTS | CPUID_ACPI |
-            CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
+            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_ACPI |
+            CPUID_SS,
+        /* Missing: CPUID_EXT_EST, CPUID_EXT_TM2 , CPUID_EXT_XTPR,
+         * CPUID_EXT_PDCM */
         .features[FEAT_1_ECX] =
-            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX |
-            CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_NX,
         .xlevel = 0x80000008,
@@ -873,14 +878,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 6,
         .model = 28,
         .stepping = 2,
+        /* Missing: CPUID_DTS, CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
-            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_VME | CPUID_DTS |
-            CPUID_ACPI | CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
+            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_VME |
+            CPUID_ACPI | CPUID_SS,
             /* Some CPUs got no CPUID_SEP */
+        /* Missing: CPUID_EXT_DSCPL, CPUID_EXT_EST, CPUID_EXT_TM2,
+         * CPUID_EXT_XTPR */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR |
             CPUID_EXT_MOVBE,
         .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 4/6] target-i386: Remove unsupported bits from all CPU models
@ 2014-08-25 20:45   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

The following CPU features were never supported by neither TCG or KVM,
so they are useless on the CPU model definitions, today:

 * CPUID_DTS (DS)
 * CPUID_HT
 * CPUID_TM
 * CPUID_PBE
 * CPUID_EXT_DTES64
 * CPUID_EXT_DSCPL
 * CPUID_EXT_EST
 * CPUID_EXT_TM2
 * CPUID_EXT_XTPR
 * CPUID_EXT_PDCM
 * CPUID_SVM_LBRV

As using "enforce" mode is the only way to ensure guest ABI doesn't
change when moving to a different host, we should make "enforce" mode
the default or at least encourage management software to always use it.

In turn, to make "enforce" usable, we need CPU models that work without
always requiring some features to be explicitly disabled. This patch
removes the above features from all CPU model definitions.

We won't need any machine-type compat code for those changes, because it
is impossible to have existing VMs with those features enabled.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
Changes v1 -> v2:
* Trivial typo fix in comment
---
 target-i386/cpu.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b7fc6e0..6f26169 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -680,10 +680,11 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 16,
         .model = 2,
         .stepping = 3,
+        /* Missing: CPUID_HT */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
-            CPUID_PSE36 | CPUID_VME | CPUID_HT,
+            CPUID_PSE36 | CPUID_VME,
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
             CPUID_EXT_POPCNT,
@@ -699,8 +700,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
+        /* Missing: CPUID_SVM_LBRV */
         .features[FEAT_SVM] =
-            CPUID_SVM_NPT | CPUID_SVM_LBRV,
+            CPUID_SVM_NPT,
         .xlevel = 0x8000001A,
         .model_id = "AMD Phenom(tm) 9550 Quad-Core Processor"
     },
@@ -711,15 +713,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 6,
         .model = 15,
         .stepping = 11,
+        /* Missing: CPUID_DTS, CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
-            CPUID_PSE36 | CPUID_VME | CPUID_DTS | CPUID_ACPI | CPUID_SS |
-            CPUID_HT | CPUID_TM | CPUID_PBE,
+            CPUID_PSE36 | CPUID_VME | CPUID_ACPI | CPUID_SS,
+        /* Missing: CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_EST,
+         * CPUID_EXT_TM2, CPUID_EXT_XTPR, CPUID_EXT_PDCM */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_DTES64 | CPUID_EXT_DSCPL | CPUID_EXT_VMX | CPUID_EXT_EST |
-            CPUID_EXT_TM2 | CPUID_EXT_CX16 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
+            CPUID_EXT_VMX | CPUID_EXT_CX16,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
@@ -794,13 +797,15 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 6,
         .model = 14,
         .stepping = 8,
+        /* Missing: CPUID_DTS, CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES | CPUID_VME |
-            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_DTS | CPUID_ACPI |
-            CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
+            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_ACPI |
+            CPUID_SS,
+        /* Missing: CPUID_EXT_EST, CPUID_EXT_TM2 , CPUID_EXT_XTPR,
+         * CPUID_EXT_PDCM */
         .features[FEAT_1_ECX] =
-            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX |
-            CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_NX,
         .xlevel = 0x80000008,
@@ -873,14 +878,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .family = 6,
         .model = 28,
         .stepping = 2,
+        /* Missing: CPUID_DTS, CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
-            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_VME | CPUID_DTS |
-            CPUID_ACPI | CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
+            CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_VME |
+            CPUID_ACPI | CPUID_SS,
             /* Some CPUs got no CPUID_SEP */
+        /* Missing: CPUID_EXT_DSCPL, CPUID_EXT_EST, CPUID_EXT_TM2,
+         * CPUID_EXT_XTPR */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR |
             CPUID_EXT_MOVBE,
         .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 5/6] target-i386: Don't enable nested VMX by default
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-25 20:45   ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

TCG doesn't support VMX, and nested VMX is not enabled by default on the
KVM kernel module.

So, there's no reason to have VMX enabled by default on the core2duo and
coreduo CPU models, today. Even the newer Intel CPU model definitions
don't have it enabled.

In this case, we need machine-type compat code, as people may be running
the older machine-types on hosts that had VMX nesting enabled.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 2 ++
 hw/i386/pc_q35.c  | 2 ++
 target-i386/cpu.c | 8 ++++----
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6ee8dfa..c6db762 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -304,6 +304,8 @@ static void pc_init_pci(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
+    x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 55fc62f..be84352 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -278,6 +278,8 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
+    x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6f26169..011316d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -719,10 +719,10 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36 | CPUID_VME | CPUID_ACPI | CPUID_SS,
         /* Missing: CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_EST,
-         * CPUID_EXT_TM2, CPUID_EXT_XTPR, CPUID_EXT_PDCM */
+         * CPUID_EXT_TM2, CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_VMX */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_VMX | CPUID_EXT_CX16,
+            CPUID_EXT_CX16,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
@@ -803,9 +803,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_ACPI |
             CPUID_SS,
         /* Missing: CPUID_EXT_EST, CPUID_EXT_TM2 , CPUID_EXT_XTPR,
-         * CPUID_EXT_PDCM */
+         * CPUID_EXT_PDCM, CPUID_EXT_VMX */
         .features[FEAT_1_ECX] =
-            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX,
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_NX,
         .xlevel = 0x80000008,
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 5/6] target-i386: Don't enable nested VMX by default
@ 2014-08-25 20:45   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

TCG doesn't support VMX, and nested VMX is not enabled by default on the
KVM kernel module.

So, there's no reason to have VMX enabled by default on the core2duo and
coreduo CPU models, today. Even the newer Intel CPU model definitions
don't have it enabled.

In this case, we need machine-type compat code, as people may be running
the older machine-types on hosts that had VMX nesting enabled.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 2 ++
 hw/i386/pc_q35.c  | 2 ++
 target-i386/cpu.c | 8 ++++----
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6ee8dfa..c6db762 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -304,6 +304,8 @@ static void pc_init_pci(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
+    x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 55fc62f..be84352 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -278,6 +278,8 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
+    x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6f26169..011316d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -719,10 +719,10 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36 | CPUID_VME | CPUID_ACPI | CPUID_SS,
         /* Missing: CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_EST,
-         * CPUID_EXT_TM2, CPUID_EXT_XTPR, CPUID_EXT_PDCM */
+         * CPUID_EXT_TM2, CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_VMX */
         .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
-            CPUID_EXT_VMX | CPUID_EXT_CX16,
+            CPUID_EXT_CX16,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         .features[FEAT_8000_0001_ECX] =
@@ -803,9 +803,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_ACPI |
             CPUID_SS,
         /* Missing: CPUID_EXT_EST, CPUID_EXT_TM2 , CPUID_EXT_XTPR,
-         * CPUID_EXT_PDCM */
+         * CPUID_EXT_PDCM, CPUID_EXT_VMX */
         .features[FEAT_1_ECX] =
-            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX,
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR,
         .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_NX,
         .xlevel = 0x80000008,
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 6/6] target-i386: Disable SVM by default in KVM mode
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-25 20:45   ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: kvm, Paolo Bonzini, Aurelien Jarno

Make SVM be disabled by default on all CPU models when in KVM mode.
Nested SVM is enabled by default in the KVM kernel module, but it is
probably less stable than nested VMX (which is already disabled by
default).

Add a new compat function, x86_cpu_compat_kvm_no_autodisable(), to keep
compatibility on previous machine-types.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 target-i386/cpu.c | 6 ++++++
 target-i386/cpu.h | 1 +
 4 files changed, 9 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6db762..87f5b81 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -306,6 +306,7 @@ static void pc_compat_2_1(MachineState *machine)
 {
     x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
     x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index be84352..5736f8a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -280,6 +280,7 @@ static void pc_compat_2_1(MachineState *machine)
 {
     x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
     x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 011316d..d3f40f5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -463,6 +463,7 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = {
 static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
     [FEAT_1_EDX] = CPUID_ACPI,
     [FEAT_1_ECX] = CPUID_EXT_MONITOR,
+    [FEAT_8000_0001_ECX] = CPUID_EXT3_SVM,
 };
 
 void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
@@ -470,6 +471,11 @@ void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
     kvm_default_features[w] &= ~features;
 }
 
+void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features)
+{
+    kvm_default_unset_features[w] &= ~features;
+}
+
 /*
  * Returns the set of feature flags that are supported and migratable by
  * QEMU, for a given FeatureWord.
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 346eac1..f496571 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1301,6 +1301,7 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
 
 void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
+void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH v2 6/6] target-i386: Disable SVM by default in KVM mode
@ 2014-08-25 20:45   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-25 20:45 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, kvm

Make SVM be disabled by default on all CPU models when in KVM mode.
Nested SVM is enabled by default in the KVM kernel module, but it is
probably less stable than nested VMX (which is already disabled by
default).

Add a new compat function, x86_cpu_compat_kvm_no_autodisable(), to keep
compatibility on previous machine-types.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 target-i386/cpu.c | 6 ++++++
 target-i386/cpu.h | 1 +
 4 files changed, 9 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6db762..87f5b81 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -306,6 +306,7 @@ static void pc_compat_2_1(MachineState *machine)
 {
     x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
     x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index be84352..5736f8a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -280,6 +280,7 @@ static void pc_compat_2_1(MachineState *machine)
 {
     x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
     x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
+    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 011316d..d3f40f5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -463,6 +463,7 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = {
 static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
     [FEAT_1_EDX] = CPUID_ACPI,
     [FEAT_1_ECX] = CPUID_EXT_MONITOR,
+    [FEAT_8000_0001_ECX] = CPUID_EXT3_SVM,
 };
 
 void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
@@ -470,6 +471,11 @@ void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
     kvm_default_features[w] &= ~features;
 }
 
+void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features)
+{
+    kvm_default_unset_features[w] &= ~features;
+}
+
 /*
  * Returns the set of feature flags that are supported and migratable by
  * QEMU, for a given FeatureWord.
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 346eac1..f496571 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1301,6 +1301,7 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
                                  uint32_t feat_add, uint32_t feat_remove);
 
 void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
+void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-26 12:56   ` Paolo Bonzini
  -1 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-26 12:56 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel, Andreas Färber; +Cc: kvm, Aurelien Jarno

Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
> 
> TCG users expect the default CPU model to contain most TCG-supported features
> (and it makes sense). See, for example, commit
> f1e00a9cf326acc1f2386a72525af8859852e1df.

It doesn't though (SMAP is the most egregious omission, and probably the
main reason why people use QEMU TCG these days), and it raises the
question of backwards-compatibility of qemu64---should we disable TCG
features in old machine types?  Probably yes, but we've never done that.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-26 12:56   ` Paolo Bonzini
  0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-26 12:56 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel, Andreas Färber; +Cc: Aurelien Jarno, kvm

Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
> 
> TCG users expect the default CPU model to contain most TCG-supported features
> (and it makes sense). See, for example, commit
> f1e00a9cf326acc1f2386a72525af8859852e1df.

It doesn't though (SMAP is the most egregious omission, and probably the
main reason why people use QEMU TCG these days), and it raises the
question of backwards-compatibility of qemu64---should we disable TCG
features in old machine types?  Probably yes, but we've never done that.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-26 12:56   ` [Qemu-devel] " Paolo Bonzini
@ 2014-08-26 18:01     ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-26 18:01 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Andreas Färber, kvm, Aurelien Jarno

On Tue, Aug 26, 2014 at 02:56:21PM +0200, Paolo Bonzini wrote:
> Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
> > 
> > TCG users expect the default CPU model to contain most TCG-supported features
> > (and it makes sense). See, for example, commit
> > f1e00a9cf326acc1f2386a72525af8859852e1df.
> 
> It doesn't though (SMAP is the most egregious omission, and probably the
> main reason why people use QEMU TCG these days), and it raises the
> question of backwards-compatibility of qemu64---should we disable TCG
> features in old machine types?  Probably yes, but we've never done that.

Had we changed qemu64, any changes to the feature set of qemu64 would
probably require compatibility code on old machine-types for KVM,
anyway. But the last time qemu64 was changed was in 2009 (commit
f1e00a9cf326acc1f2386a72525af8859852e1df), it looks like everybody was
afraid of touching "qemu64" because its purpose was not very clear.

So maybe that's good news, as things can be simpler if we make both TCG
and KVM have similar behavior:

* qemu64: a conservative default that should work out of the box on
  most systems, for both TCG and KVM. That's already the current status,
  we just need to document it.
* -cpu host: for people who want every possible feature to be enabled
  (but without cross-version live-migration support). We can easily add
  support for "-cpu host" to TCG, too.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-26 18:01     ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-26 18:01 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Aurelien Jarno, qemu-devel, kvm, Andreas Färber

On Tue, Aug 26, 2014 at 02:56:21PM +0200, Paolo Bonzini wrote:
> Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
> > 
> > TCG users expect the default CPU model to contain most TCG-supported features
> > (and it makes sense). See, for example, commit
> > f1e00a9cf326acc1f2386a72525af8859852e1df.
> 
> It doesn't though (SMAP is the most egregious omission, and probably the
> main reason why people use QEMU TCG these days), and it raises the
> question of backwards-compatibility of qemu64---should we disable TCG
> features in old machine types?  Probably yes, but we've never done that.

Had we changed qemu64, any changes to the feature set of qemu64 would
probably require compatibility code on old machine-types for KVM,
anyway. But the last time qemu64 was changed was in 2009 (commit
f1e00a9cf326acc1f2386a72525af8859852e1df), it looks like everybody was
afraid of touching "qemu64" because its purpose was not very clear.

So maybe that's good news, as things can be simpler if we make both TCG
and KVM have similar behavior:

* qemu64: a conservative default that should work out of the box on
  most systems, for both TCG and KVM. That's already the current status,
  we just need to document it.
* -cpu host: for people who want every possible feature to be enabled
  (but without cross-version live-migration support). We can easily add
  support for "-cpu host" to TCG, too.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-26 18:01     ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-27 13:36       ` Paolo Bonzini
  -1 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-27 13:36 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: qemu-devel, Andreas Färber, kvm, Aurelien Jarno

Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> On Tue, Aug 26, 2014 at 02:56:21PM +0200, Paolo Bonzini wrote:
>> Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
>>>
>>> TCG users expect the default CPU model to contain most TCG-supported features
>>> (and it makes sense). See, for example, commit
>>> f1e00a9cf326acc1f2386a72525af8859852e1df.
>>
>> It doesn't though (SMAP is the most egregious omission, and probably the
>> main reason why people use QEMU TCG these days), and it raises the
>> question of backwards-compatibility of qemu64---should we disable TCG
>> features in old machine types?  Probably yes, but we've never done that.
> 
> Had we changed qemu64, any changes to the feature set of qemu64 would
> probably require compatibility code on old machine-types for KVM,
> anyway. But the last time qemu64 was changed was in 2009 (commit
> f1e00a9cf326acc1f2386a72525af8859852e1df), it looks like everybody was
> afraid of touching "qemu64" because its purpose was not very clear.
> 
> So maybe that's good news, as things can be simpler if we make both TCG
> and KVM have similar behavior:
> 
> * qemu64: a conservative default that should work out of the box on
>   most systems, for both TCG and KVM. That's already the current status,
>   we just need to document it.
> 
> * -cpu host: for people who want every possible feature to be enabled
>   (but without cross-version live-migration support). We can easily add
>   support for "-cpu host" to TCG, too.

This means that "-cpu host" has different meanings in KVM and TCG.  Is
that an advantage or a disadvantage?

If I have to choose blindly, I'd rather give different (but sane)
meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
Basically "-cpu qemu32/64" on KVM would be changed automatically to
kvm32/64.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 13:36       ` Paolo Bonzini
  0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-27 13:36 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Aurelien Jarno, qemu-devel, kvm, Andreas Färber

Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> On Tue, Aug 26, 2014 at 02:56:21PM +0200, Paolo Bonzini wrote:
>> Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
>>>
>>> TCG users expect the default CPU model to contain most TCG-supported features
>>> (and it makes sense). See, for example, commit
>>> f1e00a9cf326acc1f2386a72525af8859852e1df.
>>
>> It doesn't though (SMAP is the most egregious omission, and probably the
>> main reason why people use QEMU TCG these days), and it raises the
>> question of backwards-compatibility of qemu64---should we disable TCG
>> features in old machine types?  Probably yes, but we've never done that.
> 
> Had we changed qemu64, any changes to the feature set of qemu64 would
> probably require compatibility code on old machine-types for KVM,
> anyway. But the last time qemu64 was changed was in 2009 (commit
> f1e00a9cf326acc1f2386a72525af8859852e1df), it looks like everybody was
> afraid of touching "qemu64" because its purpose was not very clear.
> 
> So maybe that's good news, as things can be simpler if we make both TCG
> and KVM have similar behavior:
> 
> * qemu64: a conservative default that should work out of the box on
>   most systems, for both TCG and KVM. That's already the current status,
>   we just need to document it.
> 
> * -cpu host: for people who want every possible feature to be enabled
>   (but without cross-version live-migration support). We can easily add
>   support for "-cpu host" to TCG, too.

This means that "-cpu host" has different meanings in KVM and TCG.  Is
that an advantage or a disadvantage?

If I have to choose blindly, I'd rather give different (but sane)
meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
Basically "-cpu qemu32/64" on KVM would be changed automatically to
kvm32/64.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 13:36       ` [Qemu-devel] " Paolo Bonzini
@ 2014-08-27 14:05         ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 14:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Andreas Färber, kvm, Aurelien Jarno

On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> > On Tue, Aug 26, 2014 at 02:56:21PM +0200, Paolo Bonzini wrote:
> >> Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
> >>>
> >>> TCG users expect the default CPU model to contain most TCG-supported features
> >>> (and it makes sense). See, for example, commit
> >>> f1e00a9cf326acc1f2386a72525af8859852e1df.
> >>
> >> It doesn't though (SMAP is the most egregious omission, and probably the
> >> main reason why people use QEMU TCG these days), and it raises the
> >> question of backwards-compatibility of qemu64---should we disable TCG
> >> features in old machine types?  Probably yes, but we've never done that.
> > 
> > Had we changed qemu64, any changes to the feature set of qemu64 would
> > probably require compatibility code on old machine-types for KVM,
> > anyway. But the last time qemu64 was changed was in 2009 (commit
> > f1e00a9cf326acc1f2386a72525af8859852e1df), it looks like everybody was
> > afraid of touching "qemu64" because its purpose was not very clear.
> > 
> > So maybe that's good news, as things can be simpler if we make both TCG
> > and KVM have similar behavior:
> > 
> > * qemu64: a conservative default that should work out of the box on
> >   most systems, for both TCG and KVM. That's already the current status,
> >   we just need to document it.
> > 
> > * -cpu host: for people who want every possible feature to be enabled
> >   (but without cross-version live-migration support). We can easily add
> >   support for "-cpu host" to TCG, too.
> 
> This means that "-cpu host" has different meanings in KVM and TCG.  Is
> that an advantage or a disadvantage?

It is the same meaning to me: "enable everything that's possible,
considering what's provided by the underlying accelerator". The "host"
name is misleading, though, because on KVM it is close to the host CPU,
but on TCG it depends solely on TCG's capabilities.

> 
> If I have to choose blindly, I'd rather give different (but sane)
> meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
> Basically "-cpu qemu32/64" on KVM would be changed automatically to
> kvm32/64.

This (different meanings to qemu64) is what I was proposing first,
except for the "same meaning to -cpu host" part. What exactly would you
expect "-cpu host" to mean on TCG?

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 14:05         ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 14:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Aurelien Jarno, qemu-devel, kvm, Andreas Färber

On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> > On Tue, Aug 26, 2014 at 02:56:21PM +0200, Paolo Bonzini wrote:
> >> Il 25/08/2014 22:45, Eduardo Habkost ha scritto:
> >>>
> >>> TCG users expect the default CPU model to contain most TCG-supported features
> >>> (and it makes sense). See, for example, commit
> >>> f1e00a9cf326acc1f2386a72525af8859852e1df.
> >>
> >> It doesn't though (SMAP is the most egregious omission, and probably the
> >> main reason why people use QEMU TCG these days), and it raises the
> >> question of backwards-compatibility of qemu64---should we disable TCG
> >> features in old machine types?  Probably yes, but we've never done that.
> > 
> > Had we changed qemu64, any changes to the feature set of qemu64 would
> > probably require compatibility code on old machine-types for KVM,
> > anyway. But the last time qemu64 was changed was in 2009 (commit
> > f1e00a9cf326acc1f2386a72525af8859852e1df), it looks like everybody was
> > afraid of touching "qemu64" because its purpose was not very clear.
> > 
> > So maybe that's good news, as things can be simpler if we make both TCG
> > and KVM have similar behavior:
> > 
> > * qemu64: a conservative default that should work out of the box on
> >   most systems, for both TCG and KVM. That's already the current status,
> >   we just need to document it.
> > 
> > * -cpu host: for people who want every possible feature to be enabled
> >   (but without cross-version live-migration support). We can easily add
> >   support for "-cpu host" to TCG, too.
> 
> This means that "-cpu host" has different meanings in KVM and TCG.  Is
> that an advantage or a disadvantage?

It is the same meaning to me: "enable everything that's possible,
considering what's provided by the underlying accelerator". The "host"
name is misleading, though, because on KVM it is close to the host CPU,
but on TCG it depends solely on TCG's capabilities.

> 
> If I have to choose blindly, I'd rather give different (but sane)
> meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
> Basically "-cpu qemu32/64" on KVM would be changed automatically to
> kvm32/64.

This (different meanings to qemu64) is what I was proposing first,
except for the "same meaning to -cpu host" part. What exactly would you
expect "-cpu host" to mean on TCG?

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 14:05         ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-27 14:33           ` Paolo Bonzini
  -1 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-27 14:33 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Aurelien Jarno, qemu-devel, kvm, Andreas Färber

Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
>>> So maybe that's good news, as things can be simpler if we make both TCG
>>> and KVM have similar behavior:
>>>
>>> * qemu64: a conservative default that should work out of the box on
>>>   most systems, for both TCG and KVM. That's already the current status,
>>>   we just need to document it.
>>>
>>> * -cpu host: for people who want every possible feature to be enabled
>>>   (but without cross-version live-migration support). We can easily add
>>>   support for "-cpu host" to TCG, too.
>>
>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
>> that an advantage or a disadvantage?
> 
> It is the same meaning to me: "enable everything that's possible,
> considering what's provided by the underlying accelerator". The "host"
> name is misleading, though, because on KVM it is close to the host CPU,
> but on TCG it depends solely on TCG's capabilities.

True.  It's not very intuitive, but it is the same concept for processor
capabilities.

Though for some leaves that do not correspond to processor capabilities,
"-cpu host" does set them to the host values.  This is not just the
cache model, but also the family/model/stepping/vendor.

For the TCG case, when running on a Nehalem it would be weird to see a
Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
work to have SVM with an Intel vendor. :)

>> If I have to choose blindly, I'd rather give different (but sane)
>> meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
>> Basically "-cpu qemu32/64" on KVM would be changed automatically to
>> kvm32/64.
> 
> This (different meanings to qemu64) is what I was proposing first,

Good.

> except for the "same meaning to -cpu host" part. What exactly would you
> expect "-cpu host" to mean on TCG?

Emulate (as much as possible of) a SandyBridge if I'm running on a
SandyBridge, etc.

"-cpu qemu64" would be the best CPU that TCG can do, with a standard
family/model/stepping/vendor slapped on top.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 14:33           ` Paolo Bonzini
  0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-27 14:33 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Aurelien Jarno, qemu-devel, kvm, Andreas Färber

Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
>>> So maybe that's good news, as things can be simpler if we make both TCG
>>> and KVM have similar behavior:
>>>
>>> * qemu64: a conservative default that should work out of the box on
>>>   most systems, for both TCG and KVM. That's already the current status,
>>>   we just need to document it.
>>>
>>> * -cpu host: for people who want every possible feature to be enabled
>>>   (but without cross-version live-migration support). We can easily add
>>>   support for "-cpu host" to TCG, too.
>>
>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
>> that an advantage or a disadvantage?
> 
> It is the same meaning to me: "enable everything that's possible,
> considering what's provided by the underlying accelerator". The "host"
> name is misleading, though, because on KVM it is close to the host CPU,
> but on TCG it depends solely on TCG's capabilities.

True.  It's not very intuitive, but it is the same concept for processor
capabilities.

Though for some leaves that do not correspond to processor capabilities,
"-cpu host" does set them to the host values.  This is not just the
cache model, but also the family/model/stepping/vendor.

For the TCG case, when running on a Nehalem it would be weird to see a
Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
work to have SVM with an Intel vendor. :)

>> If I have to choose blindly, I'd rather give different (but sane)
>> meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
>> Basically "-cpu qemu32/64" on KVM would be changed automatically to
>> kvm32/64.
> 
> This (different meanings to qemu64) is what I was proposing first,

Good.

> except for the "same meaning to -cpu host" part. What exactly would you
> expect "-cpu host" to mean on TCG?

Emulate (as much as possible of) a SandyBridge if I'm running on a
SandyBridge, etc.

"-cpu qemu64" would be the best CPU that TCG can do, with a standard
family/model/stepping/vendor slapped on top.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 14:33           ` [Qemu-devel] " Paolo Bonzini
@ 2014-08-27 15:42             ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 15:42 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Andreas Färber, kvm, Aurelien Jarno

On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
> > On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
> >> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> >>> So maybe that's good news, as things can be simpler if we make both TCG
> >>> and KVM have similar behavior:
> >>>
> >>> * qemu64: a conservative default that should work out of the box on
> >>>   most systems, for both TCG and KVM. That's already the current status,
> >>>   we just need to document it.
> >>>
> >>> * -cpu host: for people who want every possible feature to be enabled
> >>>   (but without cross-version live-migration support). We can easily add
> >>>   support for "-cpu host" to TCG, too.
> >>
> >> This means that "-cpu host" has different meanings in KVM and TCG.  Is
> >> that an advantage or a disadvantage?
> > 
> > It is the same meaning to me: "enable everything that's possible,
> > considering what's provided by the underlying accelerator". The "host"
> > name is misleading, though, because on KVM it is close to the host CPU,
> > but on TCG it depends solely on TCG's capabilities.
> 
> True.  It's not very intuitive, but it is the same concept for processor
> capabilities.
> 
> Though for some leaves that do not correspond to processor capabilities,
> "-cpu host" does set them to the host values.  This is not just the
> cache model, but also the family/model/stepping/vendor.
> 
> For the TCG case, when running on a Nehalem it would be weird to see a
> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
> work to have SVM with an Intel vendor. :)

In that case, the best family/model/stepping/vendor choice depends on
TCG capabilities (defined at compile time), not on the host CPU.

...and that proves your point: if we aren't even using the host CPU
family/model/stepping, calling it "-cpu host" doesn't make much sense.
If it is so different from the host model, we can call it "qemu64" (and
do as you suggests below).


> 
> >> If I have to choose blindly, I'd rather give different (but sane)
> >> meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
> >> Basically "-cpu qemu32/64" on KVM would be changed automatically to
> >> kvm32/64.
> > 
> > This (different meanings to qemu64) is what I was proposing first,
> 
> Good.
> 
> > except for the "same meaning to -cpu host" part. What exactly would you
> > expect "-cpu host" to mean on TCG?
> 
> Emulate (as much as possible of) a SandyBridge if I'm running on a
> SandyBridge, etc.
> 
> "-cpu qemu64" would be the best CPU that TCG can do, with a standard
> family/model/stepping/vendor slapped on top.

Makes sense to me.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 15:42             ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 15:42 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Aurelien Jarno, qemu-devel, kvm, Andreas Färber

On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
> > On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
> >> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> >>> So maybe that's good news, as things can be simpler if we make both TCG
> >>> and KVM have similar behavior:
> >>>
> >>> * qemu64: a conservative default that should work out of the box on
> >>>   most systems, for both TCG and KVM. That's already the current status,
> >>>   we just need to document it.
> >>>
> >>> * -cpu host: for people who want every possible feature to be enabled
> >>>   (but without cross-version live-migration support). We can easily add
> >>>   support for "-cpu host" to TCG, too.
> >>
> >> This means that "-cpu host" has different meanings in KVM and TCG.  Is
> >> that an advantage or a disadvantage?
> > 
> > It is the same meaning to me: "enable everything that's possible,
> > considering what's provided by the underlying accelerator". The "host"
> > name is misleading, though, because on KVM it is close to the host CPU,
> > but on TCG it depends solely on TCG's capabilities.
> 
> True.  It's not very intuitive, but it is the same concept for processor
> capabilities.
> 
> Though for some leaves that do not correspond to processor capabilities,
> "-cpu host" does set them to the host values.  This is not just the
> cache model, but also the family/model/stepping/vendor.
> 
> For the TCG case, when running on a Nehalem it would be weird to see a
> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
> work to have SVM with an Intel vendor. :)

In that case, the best family/model/stepping/vendor choice depends on
TCG capabilities (defined at compile time), not on the host CPU.

...and that proves your point: if we aren't even using the host CPU
family/model/stepping, calling it "-cpu host" doesn't make much sense.
If it is so different from the host model, we can call it "qemu64" (and
do as you suggests below).


> 
> >> If I have to choose blindly, I'd rather give different (but sane)
> >> meanings to "-cpu qemu64" and the same meanings to "-cpu host"...
> >> Basically "-cpu qemu32/64" on KVM would be changed automatically to
> >> kvm32/64.
> > 
> > This (different meanings to qemu64) is what I was proposing first,
> 
> Good.
> 
> > except for the "same meaning to -cpu host" part. What exactly would you
> > expect "-cpu host" to mean on TCG?
> 
> Emulate (as much as possible of) a SandyBridge if I'm running on a
> SandyBridge, etc.
> 
> "-cpu qemu64" would be the best CPU that TCG can do, with a standard
> family/model/stepping/vendor slapped on top.

Makes sense to me.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 15:42             ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-27 15:58               ` Andreas Färber
  -1 siblings, 0 replies; 36+ messages in thread
From: Andreas Färber @ 2014-08-27 15:58 UTC (permalink / raw)
  To: Eduardo Habkost, Paolo Bonzini; +Cc: qemu-devel, kvm, Aurelien Jarno

Am 27.08.2014 17:42, schrieb Eduardo Habkost:
> On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
>> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
>>> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
>>>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
>>>>> So maybe that's good news, as things can be simpler if we make both TCG
>>>>> and KVM have similar behavior:
>>>>>
>>>>> * qemu64: a conservative default that should work out of the box on
>>>>>   most systems, for both TCG and KVM. That's already the current status,
>>>>>   we just need to document it.
>>>>>
>>>>> * -cpu host: for people who want every possible feature to be enabled
>>>>>   (but without cross-version live-migration support). We can easily add
>>>>>   support for "-cpu host" to TCG, too.
>>>>
>>>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
>>>> that an advantage or a disadvantage?
>>>
>>> It is the same meaning to me: "enable everything that's possible,
>>> considering what's provided by the underlying accelerator". The "host"
>>> name is misleading, though, because on KVM it is close to the host CPU,
>>> but on TCG it depends solely on TCG's capabilities.
>>
>> True.  It's not very intuitive, but it is the same concept for processor
>> capabilities.
>>
>> Though for some leaves that do not correspond to processor capabilities,
>> "-cpu host" does set them to the host values.  This is not just the
>> cache model, but also the family/model/stepping/vendor.
>>
>> For the TCG case, when running on a Nehalem it would be weird to see a
>> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
>> work to have SVM with an Intel vendor. :)
> 
> In that case, the best family/model/stepping/vendor choice depends on
> TCG capabilities (defined at compile time), not on the host CPU.
> 
> ...and that proves your point: if we aren't even using the host CPU
> family/model/stepping, calling it "-cpu host" doesn't make much sense.
> If it is so different from the host model, we can call it "qemu64" (and
> do as you suggests below).

Might that be an opportunity to reconsider a -cpu best or so,
independent of its implementation, to avoid "host"?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 15:58               ` Andreas Färber
  0 siblings, 0 replies; 36+ messages in thread
From: Andreas Färber @ 2014-08-27 15:58 UTC (permalink / raw)
  To: Eduardo Habkost, Paolo Bonzini; +Cc: Aurelien Jarno, qemu-devel, kvm

Am 27.08.2014 17:42, schrieb Eduardo Habkost:
> On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
>> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
>>> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
>>>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
>>>>> So maybe that's good news, as things can be simpler if we make both TCG
>>>>> and KVM have similar behavior:
>>>>>
>>>>> * qemu64: a conservative default that should work out of the box on
>>>>>   most systems, for both TCG and KVM. That's already the current status,
>>>>>   we just need to document it.
>>>>>
>>>>> * -cpu host: for people who want every possible feature to be enabled
>>>>>   (but without cross-version live-migration support). We can easily add
>>>>>   support for "-cpu host" to TCG, too.
>>>>
>>>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
>>>> that an advantage or a disadvantage?
>>>
>>> It is the same meaning to me: "enable everything that's possible,
>>> considering what's provided by the underlying accelerator". The "host"
>>> name is misleading, though, because on KVM it is close to the host CPU,
>>> but on TCG it depends solely on TCG's capabilities.
>>
>> True.  It's not very intuitive, but it is the same concept for processor
>> capabilities.
>>
>> Though for some leaves that do not correspond to processor capabilities,
>> "-cpu host" does set them to the host values.  This is not just the
>> cache model, but also the family/model/stepping/vendor.
>>
>> For the TCG case, when running on a Nehalem it would be weird to see a
>> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
>> work to have SVM with an Intel vendor. :)
> 
> In that case, the best family/model/stepping/vendor choice depends on
> TCG capabilities (defined at compile time), not on the host CPU.
> 
> ...and that proves your point: if we aren't even using the host CPU
> family/model/stepping, calling it "-cpu host" doesn't make much sense.
> If it is so different from the host model, we can call it "qemu64" (and
> do as you suggests below).

Might that be an opportunity to reconsider a -cpu best or so,
independent of its implementation, to avoid "host"?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 15:58               ` [Qemu-devel] " Andreas Färber
@ 2014-08-27 16:08                 ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 16:08 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Paolo Bonzini, qemu-devel, kvm, Aurelien Jarno

On Wed, Aug 27, 2014 at 05:58:49PM +0200, Andreas Färber wrote:
> Am 27.08.2014 17:42, schrieb Eduardo Habkost:
> > On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
> >> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
> >>> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
> >>>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> >>>>> So maybe that's good news, as things can be simpler if we make both TCG
> >>>>> and KVM have similar behavior:
> >>>>>
> >>>>> * qemu64: a conservative default that should work out of the box on
> >>>>>   most systems, for both TCG and KVM. That's already the current status,
> >>>>>   we just need to document it.
> >>>>>
> >>>>> * -cpu host: for people who want every possible feature to be enabled
> >>>>>   (but without cross-version live-migration support). We can easily add
> >>>>>   support for "-cpu host" to TCG, too.
> >>>>
> >>>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
> >>>> that an advantage or a disadvantage?
> >>>
> >>> It is the same meaning to me: "enable everything that's possible,
> >>> considering what's provided by the underlying accelerator". The "host"
> >>> name is misleading, though, because on KVM it is close to the host CPU,
> >>> but on TCG it depends solely on TCG's capabilities.
> >>
> >> True.  It's not very intuitive, but it is the same concept for processor
> >> capabilities.
> >>
> >> Though for some leaves that do not correspond to processor capabilities,
> >> "-cpu host" does set them to the host values.  This is not just the
> >> cache model, but also the family/model/stepping/vendor.
> >>
> >> For the TCG case, when running on a Nehalem it would be weird to see a
> >> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
> >> work to have SVM with an Intel vendor. :)
> > 
> > In that case, the best family/model/stepping/vendor choice depends on
> > TCG capabilities (defined at compile time), not on the host CPU.
> > 
> > ...and that proves your point: if we aren't even using the host CPU
> > family/model/stepping, calling it "-cpu host" doesn't make much sense.
> > If it is so different from the host model, we can call it "qemu64" (and
> > do as you suggests below).
> 
> Might that be an opportunity to reconsider a -cpu best or so,
> independent of its implementation, to avoid "host"?

It depends on what you expect "-cpu best" to mean. I have seen different
meanings being proposed for it.

IIRC, "best" was proposed to mean "choose the best one from the existing
(predefined) CPU models", not "enable everything possible, not even
looking at the CPU model table".

Anyway, it makes sense to have a name for the "enable everything" mode
(whatever it is), and simply make "qemu64" an alias to it when in TCG
mode.

(If we didn't have existing libvirt code assuming "qemu64" is always the
default in QEMU, we could simply get rid of "qemu64" and use better
names. We may get rid of "qemu64" later, but we need to provide a way
for libvirt to stop using it, first.)

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 16:08                 ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 16:08 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Paolo Bonzini, Aurelien Jarno, qemu-devel, kvm

On Wed, Aug 27, 2014 at 05:58:49PM +0200, Andreas Färber wrote:
> Am 27.08.2014 17:42, schrieb Eduardo Habkost:
> > On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
> >> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
> >>> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
> >>>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
> >>>>> So maybe that's good news, as things can be simpler if we make both TCG
> >>>>> and KVM have similar behavior:
> >>>>>
> >>>>> * qemu64: a conservative default that should work out of the box on
> >>>>>   most systems, for both TCG and KVM. That's already the current status,
> >>>>>   we just need to document it.
> >>>>>
> >>>>> * -cpu host: for people who want every possible feature to be enabled
> >>>>>   (but without cross-version live-migration support). We can easily add
> >>>>>   support for "-cpu host" to TCG, too.
> >>>>
> >>>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
> >>>> that an advantage or a disadvantage?
> >>>
> >>> It is the same meaning to me: "enable everything that's possible,
> >>> considering what's provided by the underlying accelerator". The "host"
> >>> name is misleading, though, because on KVM it is close to the host CPU,
> >>> but on TCG it depends solely on TCG's capabilities.
> >>
> >> True.  It's not very intuitive, but it is the same concept for processor
> >> capabilities.
> >>
> >> Though for some leaves that do not correspond to processor capabilities,
> >> "-cpu host" does set them to the host values.  This is not just the
> >> cache model, but also the family/model/stepping/vendor.
> >>
> >> For the TCG case, when running on a Nehalem it would be weird to see a
> >> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
> >> work to have SVM with an Intel vendor. :)
> > 
> > In that case, the best family/model/stepping/vendor choice depends on
> > TCG capabilities (defined at compile time), not on the host CPU.
> > 
> > ...and that proves your point: if we aren't even using the host CPU
> > family/model/stepping, calling it "-cpu host" doesn't make much sense.
> > If it is so different from the host model, we can call it "qemu64" (and
> > do as you suggests below).
> 
> Might that be an opportunity to reconsider a -cpu best or so,
> independent of its implementation, to avoid "host"?

It depends on what you expect "-cpu best" to mean. I have seen different
meanings being proposed for it.

IIRC, "best" was proposed to mean "choose the best one from the existing
(predefined) CPU models", not "enable everything possible, not even
looking at the CPU model table".

Anyway, it makes sense to have a name for the "enable everything" mode
(whatever it is), and simply make "qemu64" an alias to it when in TCG
mode.

(If we didn't have existing libvirt code assuming "qemu64" is always the
default in QEMU, we could simply get rid of "qemu64" and use better
names. We may get rid of "qemu64" later, but we need to provide a way
for libvirt to stop using it, first.)

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 16:08                 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-27 16:14                   ` Andreas Färber
  -1 siblings, 0 replies; 36+ messages in thread
From: Andreas Färber @ 2014-08-27 16:14 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, qemu-devel, kvm, Aurelien Jarno

Am 27.08.2014 18:08, schrieb Eduardo Habkost:
> On Wed, Aug 27, 2014 at 05:58:49PM +0200, Andreas Färber wrote:
>> Am 27.08.2014 17:42, schrieb Eduardo Habkost:
>>> On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
>>>> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
>>>>> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
>>>>>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
>>>>>>> So maybe that's good news, as things can be simpler if we make both TCG
>>>>>>> and KVM have similar behavior:
>>>>>>>
>>>>>>> * qemu64: a conservative default that should work out of the box on
>>>>>>>   most systems, for both TCG and KVM. That's already the current status,
>>>>>>>   we just need to document it.
>>>>>>>
>>>>>>> * -cpu host: for people who want every possible feature to be enabled
>>>>>>>   (but without cross-version live-migration support). We can easily add
>>>>>>>   support for "-cpu host" to TCG, too.
>>>>>>
>>>>>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
>>>>>> that an advantage or a disadvantage?
>>>>>
>>>>> It is the same meaning to me: "enable everything that's possible,
>>>>> considering what's provided by the underlying accelerator". The "host"
>>>>> name is misleading, though, because on KVM it is close to the host CPU,
>>>>> but on TCG it depends solely on TCG's capabilities.
>>>>
>>>> True.  It's not very intuitive, but it is the same concept for processor
>>>> capabilities.
>>>>
>>>> Though for some leaves that do not correspond to processor capabilities,
>>>> "-cpu host" does set them to the host values.  This is not just the
>>>> cache model, but also the family/model/stepping/vendor.
>>>>
>>>> For the TCG case, when running on a Nehalem it would be weird to see a
>>>> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
>>>> work to have SVM with an Intel vendor. :)
>>>
>>> In that case, the best family/model/stepping/vendor choice depends on
>>> TCG capabilities (defined at compile time), not on the host CPU.
>>>
>>> ...and that proves your point: if we aren't even using the host CPU
>>> family/model/stepping, calling it "-cpu host" doesn't make much sense.
>>> If it is so different from the host model, we can call it "qemu64" (and
>>> do as you suggests below).
>>
>> Might that be an opportunity to reconsider a -cpu best or so,
>> independent of its implementation, to avoid "host"?
> 
> It depends on what you expect "-cpu best" to mean. I have seen different
> meanings being proposed for it.
> 
> IIRC, "best" was proposed to mean "choose the best one from the existing
> (predefined) CPU models", not "enable everything possible, not even
> looking at the CPU model table".
> 
> Anyway, it makes sense to have a name for the "enable everything" mode
> (whatever it is), and simply make "qemu64" an alias to it when in TCG
> mode.
> 
> (If we didn't have existing libvirt code assuming "qemu64" is always the
> default in QEMU, we could simply get rid of "qemu64" and use better
> names. We may get rid of "qemu64" later, but we need to provide a way
> for libvirt to stop using it, first.)

My "or so" referring to, e.g., -cpu optimum or -cpu maximum or whatever
we come up with that is a little more telling than "qemu64" or "host".

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 16:14                   ` Andreas Färber
  0 siblings, 0 replies; 36+ messages in thread
From: Andreas Färber @ 2014-08-27 16:14 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Aurelien Jarno, qemu-devel, kvm

Am 27.08.2014 18:08, schrieb Eduardo Habkost:
> On Wed, Aug 27, 2014 at 05:58:49PM +0200, Andreas Färber wrote:
>> Am 27.08.2014 17:42, schrieb Eduardo Habkost:
>>> On Wed, Aug 27, 2014 at 04:33:54PM +0200, Paolo Bonzini wrote:
>>>> Il 27/08/2014 16:05, Eduardo Habkost ha scritto:
>>>>> On Wed, Aug 27, 2014 at 03:36:51PM +0200, Paolo Bonzini wrote:
>>>>>> Il 26/08/2014 20:01, Eduardo Habkost ha scritto:
>>>>>>> So maybe that's good news, as things can be simpler if we make both TCG
>>>>>>> and KVM have similar behavior:
>>>>>>>
>>>>>>> * qemu64: a conservative default that should work out of the box on
>>>>>>>   most systems, for both TCG and KVM. That's already the current status,
>>>>>>>   we just need to document it.
>>>>>>>
>>>>>>> * -cpu host: for people who want every possible feature to be enabled
>>>>>>>   (but without cross-version live-migration support). We can easily add
>>>>>>>   support for "-cpu host" to TCG, too.
>>>>>>
>>>>>> This means that "-cpu host" has different meanings in KVM and TCG.  Is
>>>>>> that an advantage or a disadvantage?
>>>>>
>>>>> It is the same meaning to me: "enable everything that's possible,
>>>>> considering what's provided by the underlying accelerator". The "host"
>>>>> name is misleading, though, because on KVM it is close to the host CPU,
>>>>> but on TCG it depends solely on TCG's capabilities.
>>>>
>>>> True.  It's not very intuitive, but it is the same concept for processor
>>>> capabilities.
>>>>
>>>> Though for some leaves that do not correspond to processor capabilities,
>>>> "-cpu host" does set them to the host values.  This is not just the
>>>> cache model, but also the family/model/stepping/vendor.
>>>>
>>>> For the TCG case, when running on a Nehalem it would be weird to see a
>>>> Nehalem guest with SMAP or ADOX support...  I'm not sure it would even
>>>> work to have SVM with an Intel vendor. :)
>>>
>>> In that case, the best family/model/stepping/vendor choice depends on
>>> TCG capabilities (defined at compile time), not on the host CPU.
>>>
>>> ...and that proves your point: if we aren't even using the host CPU
>>> family/model/stepping, calling it "-cpu host" doesn't make much sense.
>>> If it is so different from the host model, we can call it "qemu64" (and
>>> do as you suggests below).
>>
>> Might that be an opportunity to reconsider a -cpu best or so,
>> independent of its implementation, to avoid "host"?
> 
> It depends on what you expect "-cpu best" to mean. I have seen different
> meanings being proposed for it.
> 
> IIRC, "best" was proposed to mean "choose the best one from the existing
> (predefined) CPU models", not "enable everything possible, not even
> looking at the CPU model table".
> 
> Anyway, it makes sense to have a name for the "enable everything" mode
> (whatever it is), and simply make "qemu64" an alias to it when in TCG
> mode.
> 
> (If we didn't have existing libvirt code assuming "qemu64" is always the
> default in QEMU, we could simply get rid of "qemu64" and use better
> names. We may get rid of "qemu64" later, but we need to provide a way
> for libvirt to stop using it, first.)

My "or so" referring to, e.g., -cpu optimum or -cpu maximum or whatever
we come up with that is a little more telling than "qemu64" or "host".

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 16:08                 ` [Qemu-devel] " Eduardo Habkost
@ 2014-08-27 16:18                   ` Paolo Bonzini
  -1 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-27 16:18 UTC (permalink / raw)
  To: Eduardo Habkost, Andreas Färber; +Cc: qemu-devel, kvm, Aurelien Jarno

Il 27/08/2014 18:08, Eduardo Habkost ha scritto:
> > Might that be an opportunity to reconsider a -cpu best or so,
> > independent of its implementation, to avoid "host"?

Nowadays we have CPU models added way before silicon is available, and
"-cpu host" in practice should be migratable (the big exception being
nested VMX and, when running on KVM, nested SVM).  What would "-cpu
best" be useful for?

> It depends on what you expect "-cpu best" to mean. I have seen different
> meanings being proposed for it.
> 
> IIRC, "best" was proposed to mean "choose the best one from the existing
> (predefined) CPU models", not "enable everything possible, not even
> looking at the CPU model table".

How do you define "best"?  You could have a model that lacks feature F1
and a model that lacks feature F2.

Adding features on top of an existing model is what libvirt's <cpu
mode='host-model'/> element does, and it's broken.  It's broken because
some features do not work unless you also bump the level (for example
xsave, my favorite example for CPUID bugs, requires leaf 0xD to be present).

> Anyway, it makes sense to have a name for the "enable everything" mode
> (whatever it is), and simply make "qemu64" an alias to it when in TCG
> mode.

Or conversely, say "qemu64" is { baseline for KVM, enable-everything for
TCG }.  Then "-cpu best" and "-cpu qemu64" would effectively be synonyms
on TCG.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 16:18                   ` Paolo Bonzini
  0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2014-08-27 16:18 UTC (permalink / raw)
  To: Eduardo Habkost, Andreas Färber; +Cc: Aurelien Jarno, qemu-devel, kvm

Il 27/08/2014 18:08, Eduardo Habkost ha scritto:
> > Might that be an opportunity to reconsider a -cpu best or so,
> > independent of its implementation, to avoid "host"?

Nowadays we have CPU models added way before silicon is available, and
"-cpu host" in practice should be migratable (the big exception being
nested VMX and, when running on KVM, nested SVM).  What would "-cpu
best" be useful for?

> It depends on what you expect "-cpu best" to mean. I have seen different
> meanings being proposed for it.
> 
> IIRC, "best" was proposed to mean "choose the best one from the existing
> (predefined) CPU models", not "enable everything possible, not even
> looking at the CPU model table".

How do you define "best"?  You could have a model that lacks feature F1
and a model that lacks feature F2.

Adding features on top of an existing model is what libvirt's <cpu
mode='host-model'/> element does, and it's broken.  It's broken because
some features do not work unless you also bump the level (for example
xsave, my favorite example for CPUID bugs, requires leaf 0xD to be present).

> Anyway, it makes sense to have a name for the "enable everything" mode
> (whatever it is), and simply make "qemu64" an alias to it when in TCG
> mode.

Or conversely, say "qemu64" is { baseline for KVM, enable-everything for
TCG }.  Then "-cpu best" and "-cpu qemu64" would effectively be synonyms
on TCG.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
  2014-08-27 16:18                   ` [Qemu-devel] " Paolo Bonzini
@ 2014-08-27 16:34                     ` Eduardo Habkost
  -1 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 16:34 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andreas Färber, qemu-devel, kvm, Aurelien Jarno

On Wed, Aug 27, 2014 at 06:18:13PM +0200, Paolo Bonzini wrote:
> Il 27/08/2014 18:08, Eduardo Habkost ha scritto:
> > > Might that be an opportunity to reconsider a -cpu best or so,
> > > independent of its implementation, to avoid "host"?
> 
> Nowadays we have CPU models added way before silicon is available, and
> "-cpu host" in practice should be migratable (the big exception being
> nested VMX and, when running on KVM, nested SVM).  What would "-cpu
> best" be useful for?
> 
> > It depends on what you expect "-cpu best" to mean. I have seen different
> > meanings being proposed for it.
> > 
> > IIRC, "best" was proposed to mean "choose the best one from the existing
> > (predefined) CPU models", not "enable everything possible, not even
> > looking at the CPU model table".
> 
> How do you define "best"?  You could have a model that lacks feature F1
> and a model that lacks feature F2.

That's one reason we never implemented it. :)

In other words: deciding what's really "best" is not something that can
be decided by QEMU alone.

> 
> Adding features on top of an existing model is what libvirt's <cpu
> mode='host-model'/> element does, and it's broken.  It's broken because
> some features do not work unless you also bump the level (for example
> xsave, my favorite example for CPUID bugs, requires leaf 0xD to be present).

I want to fix that. We have existing code to bump level when features on
leaf 0x7 are present, and there's no reason we can't generalize that to
all features that need a specific leaf to be present. We just need to be
careful to keep backwards compatibility.

> 
> > Anyway, it makes sense to have a name for the "enable everything" mode
> > (whatever it is), and simply make "qemu64" an alias to it when in TCG
> > mode.
> 
> Or conversely, say "qemu64" is { baseline for KVM, enable-everything for
> TCG }.  Then "-cpu best" and "-cpu qemu64" would effectively be synonyms
> on TCG.

This is another way to see it. But I prefer to treat it as just a
(temporary?) alias to a meaningful model name, because the only reason
we are keeping the "qemu64" name (instead of simply making "-cpu
best/maximum/whatever" the default on TCG and "-cpu kvm64" the default
on KVM) is for compatibility with existing management code.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box
@ 2014-08-27 16:34                     ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2014-08-27 16:34 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Aurelien Jarno, Andreas Färber, kvm, qemu-devel

On Wed, Aug 27, 2014 at 06:18:13PM +0200, Paolo Bonzini wrote:
> Il 27/08/2014 18:08, Eduardo Habkost ha scritto:
> > > Might that be an opportunity to reconsider a -cpu best or so,
> > > independent of its implementation, to avoid "host"?
> 
> Nowadays we have CPU models added way before silicon is available, and
> "-cpu host" in practice should be migratable (the big exception being
> nested VMX and, when running on KVM, nested SVM).  What would "-cpu
> best" be useful for?
> 
> > It depends on what you expect "-cpu best" to mean. I have seen different
> > meanings being proposed for it.
> > 
> > IIRC, "best" was proposed to mean "choose the best one from the existing
> > (predefined) CPU models", not "enable everything possible, not even
> > looking at the CPU model table".
> 
> How do you define "best"?  You could have a model that lacks feature F1
> and a model that lacks feature F2.

That's one reason we never implemented it. :)

In other words: deciding what's really "best" is not something that can
be decided by QEMU alone.

> 
> Adding features on top of an existing model is what libvirt's <cpu
> mode='host-model'/> element does, and it's broken.  It's broken because
> some features do not work unless you also bump the level (for example
> xsave, my favorite example for CPUID bugs, requires leaf 0xD to be present).

I want to fix that. We have existing code to bump level when features on
leaf 0x7 are present, and there's no reason we can't generalize that to
all features that need a specific leaf to be present. We just need to be
careful to keep backwards compatibility.

> 
> > Anyway, it makes sense to have a name for the "enable everything" mode
> > (whatever it is), and simply make "qemu64" an alias to it when in TCG
> > mode.
> 
> Or conversely, say "qemu64" is { baseline for KVM, enable-everything for
> TCG }.  Then "-cpu best" and "-cpu qemu64" would effectively be synonyms
> on TCG.

This is another way to see it. But I prefer to treat it as just a
(temporary?) alias to a meaningful model name, because the only reason
we are keeping the "qemu64" name (instead of simply making "-cpu
best/maximum/whatever" the default on TCG and "-cpu kvm64" the default
on KVM) is for compatibility with existing management code.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2014-08-27 16:34 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-25 20:45 [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box Eduardo Habkost
2014-08-25 20:45 ` [Qemu-devel] " Eduardo Habkost
2014-08-25 20:45 ` [PATCH v2 1/6] pc: Create pc_compat_2_1() functions Eduardo Habkost
2014-08-25 20:45   ` [Qemu-devel] " Eduardo Habkost
2014-08-25 20:45 ` [PATCH v2 2/6] target-i386: Rename KVM auto-feature-enable compat function Eduardo Habkost
2014-08-25 20:45   ` [Qemu-devel] " Eduardo Habkost
2014-08-25 20:45 ` [PATCH v2 3/6] target-i386: Disable CPUID_ACPI by default on KVM mode Eduardo Habkost
2014-08-25 20:45   ` [Qemu-devel] " Eduardo Habkost
2014-08-25 20:45 ` [PATCH v2 4/6] target-i386: Remove unsupported bits from all CPU models Eduardo Habkost
2014-08-25 20:45   ` [Qemu-devel] " Eduardo Habkost
2014-08-25 20:45 ` [PATCH v2 5/6] target-i386: Don't enable nested VMX by default Eduardo Habkost
2014-08-25 20:45   ` [Qemu-devel] " Eduardo Habkost
2014-08-25 20:45 ` [PATCH v2 6/6] target-i386: Disable SVM by default in KVM mode Eduardo Habkost
2014-08-25 20:45   ` [Qemu-devel] " Eduardo Habkost
2014-08-26 12:56 ` [PATCH v2 0/6] target-i386: Make most CPU models work with "enforce" out of the box Paolo Bonzini
2014-08-26 12:56   ` [Qemu-devel] " Paolo Bonzini
2014-08-26 18:01   ` Eduardo Habkost
2014-08-26 18:01     ` [Qemu-devel] " Eduardo Habkost
2014-08-27 13:36     ` Paolo Bonzini
2014-08-27 13:36       ` [Qemu-devel] " Paolo Bonzini
2014-08-27 14:05       ` Eduardo Habkost
2014-08-27 14:05         ` [Qemu-devel] " Eduardo Habkost
2014-08-27 14:33         ` Paolo Bonzini
2014-08-27 14:33           ` [Qemu-devel] " Paolo Bonzini
2014-08-27 15:42           ` Eduardo Habkost
2014-08-27 15:42             ` [Qemu-devel] " Eduardo Habkost
2014-08-27 15:58             ` Andreas Färber
2014-08-27 15:58               ` [Qemu-devel] " Andreas Färber
2014-08-27 16:08               ` Eduardo Habkost
2014-08-27 16:08                 ` [Qemu-devel] " Eduardo Habkost
2014-08-27 16:14                 ` Andreas Färber
2014-08-27 16:14                   ` [Qemu-devel] " Andreas Färber
2014-08-27 16:18                 ` Paolo Bonzini
2014-08-27 16:18                   ` [Qemu-devel] " Paolo Bonzini
2014-08-27 16:34                   ` Eduardo Habkost
2014-08-27 16:34                     ` [Qemu-devel] " Eduardo Habkost

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.