[Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Eduardo Habkost]

The x86 change to make "check" mode be enabled by default made QEMU print a
warning in the default case if running in an Intel host:

  $ qemu-system-x86_64 -machine pc,accel=kvm
  warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]

Fix this by not enabling sse4a in qemu64 when in KVM mode.

The long term solution will probably involve creating separate "tcg64" and
"kvm64" CPU models as defaults, so we can finally choose completely diffferent
defaults in the KVM- and TCG-specific models without making the
kvm_default_props list grow too mcuh.

Eduardo Habkost (3):
  target-i386: Add optional class name to kvm_default_props
  pc: Create pc_compat_2_4() function
  target-i386: Don't enable SSE4A by default with KVM

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

-- 
2.1.0

[Qemu-devel] [PATCH 1/3] target-i386: Add optional class name to kvm_default_props

[Eduardo Habkost]

This will allow us to define class-specific KVM defaults, instead of
defaults that apply to all CPU models.

If this table is starting to look like the global properties tables,
that's not a coincidence: in the future, we might convert this to
accelerator-specific code that simply register global properties when
the machine is initialized.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 393dcc4..fd4bbc9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -325,7 +325,7 @@ static void pc_compat_2_1(MachineState *machine)
 
     pc_compat_2_2(machine);
     smbios_uuid_encoded = false;
-    x86_cpu_change_kvm_default("svm", NULL);
+    x86_cpu_change_kvm_default(NULL, "svm", NULL);
     pcms->enforce_aligned_dimm = false;
 }
 
@@ -361,7 +361,7 @@ static void pc_compat_1_7(MachineState *machine)
     gigabyte_align = false;
     option_rom_has_mr = true;
     legacy_acpi_table_size = 6414;
-    x86_cpu_change_kvm_default("x2apic", NULL);
+    x86_cpu_change_kvm_default(NULL, "x2apic", NULL);
 }
 
 static void pc_compat_1_6(MachineState *machine)
@@ -391,7 +391,7 @@ static void pc_compat_1_3(MachineState *machine)
 static void pc_compat_1_2(MachineState *machine)
 {
     pc_compat_1_3(machine);
-    x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
+    x86_cpu_change_kvm_default(NULL, "kvm-pv-eoi", NULL);
 }
 
 /* PC compat function for pc-0.10 to pc-0.13 */
@@ -414,7 +414,7 @@ static void pc_init_isa(MachineState *machine)
     if (!machine->cpu_model) {
         machine->cpu_model = "486";
     }
-    x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
+    x86_cpu_change_kvm_default(NULL, "kvm-pv-eoi", NULL);
     enable_compat_apic_id_mode();
     pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2f8f396..833b1c1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -309,7 +309,7 @@ static void pc_compat_2_1(MachineState *machine)
     pc_compat_2_2(machine);
     pcms->enforce_aligned_dimm = false;
     smbios_uuid_encoded = false;
-    x86_cpu_change_kvm_default("svm", NULL);
+    x86_cpu_change_kvm_default(NULL, "svm", NULL);
 }
 
 static void pc_compat_2_0(MachineState *machine)
@@ -326,7 +326,7 @@ static void pc_compat_1_7(MachineState *machine)
     smbios_defaults = false;
     gigabyte_align = false;
     option_rom_has_mr = true;
-    x86_cpu_change_kvm_default("x2apic", NULL);
+    x86_cpu_change_kvm_default(NULL, "x2apic", NULL);
 }
 
 static void pc_compat_1_6(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9280bfc..b699a2c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1359,6 +1359,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 };
 
 typedef struct PropValue {
+    const char *type;
     const char *prop, *value;
 } PropValue;
 
@@ -1366,24 +1367,25 @@ typedef struct PropValue {
  * from all CPU models when KVM is enabled.
  */
 static PropValue kvm_default_props[] = {
-    { "kvmclock", "on" },
-    { "kvm-nopiodelay", "on" },
-    { "kvm-asyncpf", "on" },
-    { "kvm-steal-time", "on" },
-    { "kvm-pv-eoi", "on" },
-    { "kvmclock-stable-bit", "on" },
-    { "x2apic", "on" },
-    { "acpi", "off" },
-    { "monitor", "off" },
-    { "svm", "off" },
+    { NULL, "kvmclock", "on" },
+    { NULL, "kvm-nopiodelay", "on" },
+    { NULL, "kvm-asyncpf", "on" },
+    { NULL, "kvm-steal-time", "on" },
+    { NULL, "kvm-pv-eoi", "on" },
+    { NULL, "kvmclock-stable-bit", "on" },
+    { NULL, "x2apic", "on" },
+    { NULL, "acpi", "off" },
+    { NULL, "monitor", "off" },
+    { NULL, "svm", "off" },
     { NULL, NULL },
 };
 
-void x86_cpu_change_kvm_default(const char *prop, const char *value)
+void x86_cpu_change_kvm_default(const char *type, const char *prop,
+                                const char *value)
 {
     PropValue *pv;
     for (pv = kvm_default_props; pv->prop; pv++) {
-        if (!strcmp(pv->prop, prop)) {
+        if (!g_strcmp0(pv->type, type) && !strcmp(pv->prop, prop)) {
             pv->value = value;
             break;
         }
@@ -2073,6 +2075,9 @@ static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
         if (!pv->value) {
             continue;
         }
+        if (pv->type && !object_dynamic_cast(OBJECT(cpu), pv->type)) {
+            continue;
+        }
         object_property_parse(OBJECT(cpu), pv->value, pv->prop,
                               &error_abort);
     }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 62f7879..ef57247 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1324,7 +1324,8 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
  * It is valid to call this funciton only for properties that
  * are already present in the kvm_default_props table.
  */
-void x86_cpu_change_kvm_default(const char *prop, const char *value);
+void x86_cpu_change_kvm_default(const char *type, const char *prop,
+                                const char *value);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
2.1.0

[Qemu-devel] [PATCH 2/3] pc: Create pc_compat_2_4() function

[Eduardo Habkost]

We will need to call x86_cpu_change_kvm_default() for pc-2.4 too, so we
will need a pc_compat_2_4() function.

I plan to fix this by adding a kvm_defaults list to PCMachineClass
later, but while we don't have that, let's do it the easy way and add
the new function.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fd4bbc9..d6616c4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -294,16 +294,14 @@ static void pc_init1(MachineState *machine,
     }
 }
 
-/* Looking for a pc_compat_2_4() function? It doesn't exist.
- * pc_compat_*() functions that run on machine-init time and
- * change global QEMU state are deprecated. Please don't create
- * one, and implement any pc-*-2.4 (and newer) compat code in
- * HW_COMPAT_*, PC_COMPAT_*, or * pc_*_machine_options().
- */
+static void pc_compat_2_4(MachineState *machine)
+{
+}
 
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -490,7 +488,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
-DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
+DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", pc_compat_2_4,
                       pc_i440fx_2_4_machine_options)
 
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 833b1c1..b33fcdc 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -277,16 +277,14 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
-/* Looking for a pc_compat_2_4() function? It doesn't exist.
- * pc_compat_*() functions that run on machine-init time and
- * change global QEMU state are deprecated. Please don't create
- * one, and implement any pc-*-2.4 (and newer) compat code in
- * HW_COMPAT_*, PC_COMPAT_*, or * pc_*_machine_options().
- */
+static void pc_compat_2_4(MachineState *machine)
+{
+}
 
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -388,7 +386,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
-DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
+DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", pc_compat_2_4,
                    pc_q35_2_4_machine_options);
 
 
-- 
2.1.0

[Qemu-devel] [PATCH 3/3] target-i386: Don't enable SSE4A by default with KVM

[Eduardo Habkost]

The x86 change to make "check" mode be enabled by default made QEMU print a
warning in the default case if running in an Intel host:

  $ qemu-system-x86_64 -machine pc,accel=kvm
  warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]

Fix this by not enabling sse4a in qemu64 when in KVM mode.

The long term solution will probably involve creating separate "tcg64" and
"kvm64" CPU models as defaults, so we can finally choose completely diffferent
defaults in the KVM- and TCG-specific models without making the
kvm_default_props list grow too much.

Instead of using X86_CPU_TYPE_NAME("qemu64") on kvm_default_props, the
table will contain both -x86_64-cpu and -i386 entries to make it easier
to move the table to accelerator-specific (and arch-independent) code
later.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d6616c4..a4cf6b9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -296,6 +296,8 @@ static void pc_init1(MachineState *machine,
 
 static void pc_compat_2_4(MachineState *machine)
 {
+    x86_cpu_change_kvm_default("qemu64-x86_64-cpu", "sse4a", NULL);
+    x86_cpu_change_kvm_default("qemu64-i386-cpu", "sse4a", NULL);
 }
 
 static void pc_compat_2_3(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b33fcdc..da0ed8a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -279,6 +279,8 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_compat_2_4(MachineState *machine)
 {
+    x86_cpu_change_kvm_default("qemu64-x86_64-cpu", "sse4a", NULL);
+    x86_cpu_change_kvm_default("qemu64-i386-cpu", "sse4a", NULL);
 }
 
 static void pc_compat_2_3(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b699a2c..c4d6985 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1377,6 +1377,11 @@ static PropValue kvm_default_props[] = {
     { NULL, "acpi", "off" },
     { NULL, "monitor", "off" },
     { NULL, "svm", "off" },
+    /* sse4a is not available on Intel hosts, so don't enable it
+     * in the default CPU model for KVM.
+     */
+    { "qemu64-x86_64-cpu", "sse4a", "off", },
+    { "qemu64-i386-cpu", "sse4a", "off", },
     { NULL, NULL },
 };
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Eduardo Habkost]

On Fri, Oct 30, 2015 at 05:00:57PM -0200, Eduardo Habkost wrote:
> The x86 change to make "check" mode be enabled by default made QEMU print a
> warning in the default case if running in an Intel host:
> 
>   $ qemu-system-x86_64 -machine pc,accel=kvm
>   warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
> 
> Fix this by not enabling sse4a in qemu64 when in KVM mode.
> 
> The long term solution will probably involve creating separate "tcg64" and
> "kvm64" CPU models as defaults, so we can finally choose completely diffferent
> defaults in the KVM- and TCG-specific models without making the
> kvm_default_props list grow too mcuh.

Note that we can have a much simpler solution to this: disabling SSE4A
on qemu64 completely, even on TCG. This way we can use PC_COMPAT_2_4
instead of the kvm_default_props compat hack.

We can reenable SSE4A for TCG later (more exactly, we could enable
everything from TCG_*_FEATURES) when we implement the kvm64/tcg64
solution.

What do you think?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Paolo Bonzini]

On 30/10/2015 20:13, Eduardo Habkost wrote:
> On Fri, Oct 30, 2015 at 05:00:57PM -0200, Eduardo Habkost wrote:
>> The x86 change to make "check" mode be enabled by default made QEMU print a
>> warning in the default case if running in an Intel host:
>>
>>   $ qemu-system-x86_64 -machine pc,accel=kvm
>>   warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
>>
>> Fix this by not enabling sse4a in qemu64 when in KVM mode.
>>
>> The long term solution will probably involve creating separate "tcg64" and
>> "kvm64" CPU models as defaults, so we can finally choose completely diffferent
>> defaults in the KVM- and TCG-specific models without making the
>> kvm_default_props list grow too mcuh.
> 
> Note that we can have a much simpler solution to this: disabling SSE4A
> on qemu64 completely, even on TCG. This way we can use PC_COMPAT_2_4
> instead of the kvm_default_props compat hack.
> 
> We can reenable SSE4A for TCG later (more exactly, we could enable
> everything from TCG_*_FEATURES) when we implement the kvm64/tcg64
> solution.
> 
> What do you think?

I would prefer that to this series.

Another possibility (even more of a hack, but perhaps acceptable) is to
disable SSE4A in x86_cpu_load_def if kvm_enabled() and the host vendor
is not AMD.  This is effectively what was already happening in 2.4 and
earlier.

Paolo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Eduardo Habkost]

On Mon, Nov 02, 2015 at 12:37:24PM +0100, Paolo Bonzini wrote:
> 
> 
> On 30/10/2015 20:13, Eduardo Habkost wrote:
> > On Fri, Oct 30, 2015 at 05:00:57PM -0200, Eduardo Habkost wrote:
> >> The x86 change to make "check" mode be enabled by default made QEMU print a
> >> warning in the default case if running in an Intel host:
> >>
> >>   $ qemu-system-x86_64 -machine pc,accel=kvm
> >>   warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
> >>
> >> Fix this by not enabling sse4a in qemu64 when in KVM mode.
> >>
> >> The long term solution will probably involve creating separate "tcg64" and
> >> "kvm64" CPU models as defaults, so we can finally choose completely diffferent
> >> defaults in the KVM- and TCG-specific models without making the
> >> kvm_default_props list grow too mcuh.
> > 
> > Note that we can have a much simpler solution to this: disabling SSE4A
> > on qemu64 completely, even on TCG. This way we can use PC_COMPAT_2_4
> > instead of the kvm_default_props compat hack.
> > 
> > We can reenable SSE4A for TCG later (more exactly, we could enable
> > everything from TCG_*_FEATURES) when we implement the kvm64/tcg64
> > solution.
> > 
> > What do you think?
> 
> I would prefer that to this series.
> 
> Another possibility (even more of a hack, but perhaps acceptable) is to
> disable SSE4A in x86_cpu_load_def if kvm_enabled() and the host vendor
> is not AMD.  This is effectively what was already happening in 2.4 and
> earlier.

This was happening in 2.4 and earlier only if "check" or "enforce" was
not enabled by the user.

Also, that would make the resulting feature set of "-cpu <model>" be
different depending on the host CPU vendor, in addition to being
different depending on the accelerator. I would like to avoid that,
unless there's no other way.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Paolo Bonzini]

On 03/11/2015 17:41, Eduardo Habkost wrote:
> On Mon, Nov 02, 2015 at 12:37:24PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 30/10/2015 20:13, Eduardo Habkost wrote:
>>> On Fri, Oct 30, 2015 at 05:00:57PM -0200, Eduardo Habkost wrote:
>>>> The x86 change to make "check" mode be enabled by default made QEMU print a
>>>> warning in the default case if running in an Intel host:
>>>>
>>>>   $ qemu-system-x86_64 -machine pc,accel=kvm
>>>>   warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
>>>>
>>>> Fix this by not enabling sse4a in qemu64 when in KVM mode.
>>>>
>>>> The long term solution will probably involve creating separate "tcg64" and
>>>> "kvm64" CPU models as defaults, so we can finally choose completely diffferent
>>>> defaults in the KVM- and TCG-specific models without making the
>>>> kvm_default_props list grow too mcuh.
>>>
>>> Note that we can have a much simpler solution to this: disabling SSE4A
>>> on qemu64 completely, even on TCG. This way we can use PC_COMPAT_2_4
>>> instead of the kvm_default_props compat hack.
>>>
>>> We can reenable SSE4A for TCG later (more exactly, we could enable
>>> everything from TCG_*_FEATURES) when we implement the kvm64/tcg64
>>> solution.
>>>
>>> What do you think?
>>
>> I would prefer that to this series.
>>
>> Another possibility (even more of a hack, but perhaps acceptable) is to
>> disable SSE4A in x86_cpu_load_def if kvm_enabled() and the host vendor
>> is not AMD.  This is effectively what was already happening in 2.4 and
>> earlier.
> 
> This was happening in 2.4 and earlier only if "check" or "enforce" was
> not enabled by the user.

IIUC the machine would not have started at all (in the "enforce" case)
or would have had sse4a masked anyway (in the "check" case).  So there
would be no guest ABI change.

> Also, that would make the resulting feature set of "-cpu <model>" be
> different depending on the host CPU vendor, in addition to being
> different depending on the accelerator.

It already was in the past versions, and we would have to keep the
"feature" to maintain guest ABI for past machine types, wouldn't we?
Features that KVM doesn't support are masked by x86_cpu_filter_features.

Paolo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Eduardo Habkost]

On Mon, Nov 02, 2015 at 12:37:24PM +0100, Paolo Bonzini wrote:
> On 30/10/2015 20:13, Eduardo Habkost wrote:
> > On Fri, Oct 30, 2015 at 05:00:57PM -0200, Eduardo Habkost wrote:
> >> The x86 change to make "check" mode be enabled by default made QEMU print a
> >> warning in the default case if running in an Intel host:
> >>
> >>   $ qemu-system-x86_64 -machine pc,accel=kvm
> >>   warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
> >>
> >> Fix this by not enabling sse4a in qemu64 when in KVM mode.
> >>
> >> The long term solution will probably involve creating separate "tcg64" and
> >> "kvm64" CPU models as defaults, so we can finally choose completely diffferent
> >> defaults in the KVM- and TCG-specific models without making the
> >> kvm_default_props list grow too mcuh.
> > 
> > Note that we can have a much simpler solution to this: disabling SSE4A
> > on qemu64 completely, even on TCG. This way we can use PC_COMPAT_2_4
> > instead of the kvm_default_props compat hack.
> > 
> > We can reenable SSE4A for TCG later (more exactly, we could enable
> > everything from TCG_*_FEATURES) when we implement the kvm64/tcg64
> > solution.
> > 
> > What do you think?
> 
> I would prefer that to this series.

I just got a report from David that the lack of ABM on Sandy Bridge
hosts will trigger the warning, too. I guess we don't want to require a
Haswell CPU to run the default CPU model with KVM, so ABM would be
disabled by default in KVM mode too.

We really should make the defaults different in TCG and KVM mode. The
question is what to do about the TCG defaults in QEMU 2.5.

I propose we remove ABM and SSE4E from qemu64 in 2.5, and implement a
proper mechanism to have different KVM and TCG defaults after 2.5. We
never pretended that qemu64 had all TCG-supported features enabled
anyway: the last time the feature set of qemu64 was changed was in 2009
(when we added ABM).

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Paolo Bonzini]

On 03/11/2015 18:41, Eduardo Habkost wrote:
> I just got a report from David that the lack of ABM on Sandy Bridge
> hosts will trigger the warning, too. I guess we don't want to require a
> Haswell CPU to run the default CPU model with KVM, so ABM would be
> disabled by default in KVM mode too.
> 
> We really should make the defaults different in TCG and KVM mode. The
> question is what to do about the TCG defaults in QEMU 2.5.
> 
> I propose we remove ABM and SSE4E from qemu64 in 2.5,

But if we do that for all machine types we break guest ABI for >=Haswell
on old machine types; if we do that for 2.5 only we get "-cpu check"
warnings on old machine types.

This is true even if we implement different defaults for KVM and TCG.
Should we disable "-cpu check" for old machine types (or do we already
do that)?  That would suck, but it would let us drop ABM and SSE4A from
the 2.4 machine type without much hassle.

> and implement a
> proper mechanism to have different KVM and TCG defaults after 2.5. We
> never pretended that qemu64 had all TCG-supported features enabled
> anyway: the last time the feature set of qemu64 was changed was in 2009
> (when we added ABM).

That's true.

Paolo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Eduardo Habkost]

On Tue, Nov 03, 2015 at 06:21:57PM +0100, Paolo Bonzini wrote:
> 
> 
> On 03/11/2015 17:41, Eduardo Habkost wrote:
> > On Mon, Nov 02, 2015 at 12:37:24PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 30/10/2015 20:13, Eduardo Habkost wrote:
> >>> On Fri, Oct 30, 2015 at 05:00:57PM -0200, Eduardo Habkost wrote:
> >>>> The x86 change to make "check" mode be enabled by default made QEMU print a
> >>>> warning in the default case if running in an Intel host:
> >>>>
> >>>>   $ qemu-system-x86_64 -machine pc,accel=kvm
> >>>>   warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6]
> >>>>
> >>>> Fix this by not enabling sse4a in qemu64 when in KVM mode.
> >>>>
> >>>> The long term solution will probably involve creating separate "tcg64" and
> >>>> "kvm64" CPU models as defaults, so we can finally choose completely diffferent
> >>>> defaults in the KVM- and TCG-specific models without making the
> >>>> kvm_default_props list grow too mcuh.
> >>>
> >>> Note that we can have a much simpler solution to this: disabling SSE4A
> >>> on qemu64 completely, even on TCG. This way we can use PC_COMPAT_2_4
> >>> instead of the kvm_default_props compat hack.
> >>>
> >>> We can reenable SSE4A for TCG later (more exactly, we could enable
> >>> everything from TCG_*_FEATURES) when we implement the kvm64/tcg64
> >>> solution.
> >>>
> >>> What do you think?
> >>
> >> I would prefer that to this series.
> >>
> >> Another possibility (even more of a hack, but perhaps acceptable) is to
> >> disable SSE4A in x86_cpu_load_def if kvm_enabled() and the host vendor
> >> is not AMD.  This is effectively what was already happening in 2.4 and
> >> earlier.
> > 
> > This was happening in 2.4 and earlier only if "check" or "enforce" was
> > not enabled by the user.
> 
> IIUC the machine would not have started at all (in the "enforce" case)
> or would have had sse4a masked anyway (in the "check" case).  So there
> would be no guest ABI change.
> 
> > Also, that would make the resulting feature set of "-cpu <model>" be
> > different depending on the host CPU vendor, in addition to being
> > different depending on the accelerator.
> 
> It already was in the past versions, and we would have to keep the
> "feature" to maintain guest ABI for past machine types, wouldn't we?
> Features that KVM doesn't support are masked by x86_cpu_filter_features.

Yes, it would be encoding the previous host-CPU-dependent behavior in
the code, permanently. But I would prefer to simply trigger warnings
when using "-machine pc-2.4", than having to maintain the hack in the
code forever.

(As you said, there's no ABI change, so we don't have to keep the hack
there to maintain guest ABI, but just to avoid warnings when using
pc-2.4).

BTW, we have the same problem with ABM (see my previous message) and in
that case we can't just implement a hack to disable it depending on the
host CPU.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Eduardo Habkost]

On Tue, Nov 03, 2015 at 06:49:08PM +0100, Paolo Bonzini wrote:
> 
> 
> On 03/11/2015 18:41, Eduardo Habkost wrote:
> > I just got a report from David that the lack of ABM on Sandy Bridge
> > hosts will trigger the warning, too. I guess we don't want to require a
> > Haswell CPU to run the default CPU model with KVM, so ABM would be
> > disabled by default in KVM mode too.
> > 
> > We really should make the defaults different in TCG and KVM mode. The
> > question is what to do about the TCG defaults in QEMU 2.5.
> > 
> > I propose we remove ABM and SSE4E from qemu64 in 2.5,
> 
> But if we do that for all machine types we break guest ABI for >=Haswell
> on old machine types; if we do that for 2.5 only we get "-cpu check"
> warnings on old machine types.

I don't propose we remove it from all machine-types, but just from
pc-2.5 and newer.

> 
> This is true even if we implement different defaults for KVM and TCG.
> Should we disable "-cpu check" for old machine types (or do we already
> do that)?  That would suck, but it would let us drop ABM and SSE4A from
> the 2.4 machine type without much hassle.

You mean dropping from the 2.5 machine-type, right?

> 
> > and implement a
> > proper mechanism to have different KVM and TCG defaults after 2.5. We
> > never pretended that qemu64 had all TCG-supported features enabled
> > anyway: the last time the feature set of qemu64 was changed was in 2009
> > (when we added ABM).
> 
> That's true.

So, trying to summarize what I would like to do:

* Remove ABM and SEE4A from qemu64 in pc-2.5
* Keep ABM and SSE4A in qemu64 in pc-2.4 and older
* Disable "check" by default in pc-2.4 and older, to avoid spurious
  warnings

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/3] target-i386: Don't trigger "check" warnings by default in KVM mode

[Paolo Bonzini]

On 03/11/2015 18:56, Eduardo Habkost wrote:
> So, trying to summarize what I would like to do:
> 
> * Remove ABM and SEE4A from qemu64 in pc-2.5
> * Keep ABM and SSE4A in qemu64 in pc-2.4 and older
> * Disable "check" by default in pc-2.4 and older, to avoid spurious
>   warnings

Ok, now we're on the same page.  I agree with the plan.

Paolo