* [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props
@ 2017-06-05 15:56 Eduardo Habkost
2017-06-05 16:07 ` Michael S. Tsirkin
2017-06-05 17:36 ` Eduardo Habkost
0 siblings, 2 replies; 4+ messages in thread
From: Eduardo Habkost @ 2017-06-05 15:56 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel; +Cc: Richard Henderson, Igor Mammedov
Since the automatic cpuid-level code was introduced in commit
c39c0edf9bb3b968ba95484465a50c7b19f4aa3a, the CPU model tables just
define the default CPUID level code (set using "min-level"). Setting
"[x]level" forces CPUID level to a specific value and disable the
automatic-level logic.
But the PC compat code was not updated and the existing "[x]level"
compat properties broke compatibility for people using features that
triggered the auto-level code. To keep previous behavior, we should set
"min-[x]level" instead of "[x]level" on compat_props.
This was not a problem for most cases, because old machine-types don't
have full-cpuid-auto-level enabled. The only common use case it broke
was the CPUID[7] auto-level code, that was already enabled since the
first CPUID[7] feature was introduced (in QEMU 1.4.0).
This causes the regression reported at:
https://bugzilla.redhat.com/show_bug.cgi?id=1454641
Change the PC compat code to use "min-[x]level" instead of "[x]level" on
compat_props, and add new test cases to ensure we don't break this
again.
Reported-by: "Guo, Zhiyi" <zhguo@redhat.com>
Signd-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/i386/pc.h | 42 +++++++++++++++++++++---------------------
tests/test-x86-cpuid-compat.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 21 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index e447f5d8f4..d071c9c0e9 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -566,75 +566,75 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.value = "off",\
},{\
.driver = "qemu64" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(4),\
},{\
.driver = "kvm64" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(5),\
},{\
.driver = "pentium3" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(2),\
},{\
.driver = "n270" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(5),\
},{\
.driver = "Conroe" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(4),\
},{\
.driver = "Penryn" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(4),\
},{\
.driver = "Nehalem" "-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(4),\
},{\
.driver = "n270" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Penryn" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Conroe" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Nehalem" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Westmere" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "SandyBridge" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "IvyBridge" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Haswell" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Haswell-noTSX" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Broadwell" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = "Broadwell-noTSX" "-" TYPE_X86_CPU,\
- .property = "xlevel",\
+ .property = "min-xlevel",\
.value = stringify(0x8000000a),\
},{\
.driver = TYPE_X86_CPU,\
@@ -860,7 +860,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.value = stringify(2),\
},{\
.driver = "Conroe-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(2),\
},{\
.driver = "Penryn-" TYPE_X86_CPU,\
@@ -868,7 +868,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.value = stringify(2),\
},{\
.driver = "Penryn-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(2),\
},{\
.driver = "Nehalem-" TYPE_X86_CPU,\
@@ -876,7 +876,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.value = stringify(2),\
},{\
.driver = "Nehalem-" TYPE_X86_CPU,\
- .property = "level",\
+ .property = "min-level",\
.value = stringify(2),\
},{\
.driver = "virtio-net-pci",\
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 6c71e46391..652216f531 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -313,6 +313,44 @@ int main(int argc, char **argv)
add_cpuid_test("x86/cpuid/auto-xlevel2/pc-2.7",
"-machine pc-i440fx-2.7 -cpu 486,+xstore",
"xlevel2", 0);
+ /*
+ * QEMU 1.4.0 had auto-level enabled for CPUID[7], already,
+ * and the compat code that sets default level shouldn't
+ * disable the auto-level=7 code:
+ */
+ add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-1.4/off",
+ "-machine pc-i440fx-1.4 -cpu Nehalem",
+ "level", 2);
+ add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-1.5/on",
+ "-machine pc-i440fx-1.4 -cpu Nehalem,+smap",
+ "level", 7);
+ add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/off",
+ "-machine pc-i440fx-2.3 -cpu Penryn",
+ "level", 4);
+ add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/on",
+ "-machine pc-i440fx-2.3 -cpu Penryn,+erms",
+ "level", 7);
+ add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/off",
+ "-machine pc-i440fx-2.9 -cpu Conroe",
+ "level", 10);
+ add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/on",
+ "-machine pc-i440fx-2.9 -cpu Conroe,+erms",
+ "level", 10);
+
+ /*
+ * xlevel don't have any feature that triggers auto-level
+ * code on old machine-types. Just check if the compat code
+ * is working correctly:
+ */
+ add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.3",
+ "-machine pc-i440fx-2.3 -cpu SandyBridge",
+ "xlevel", 0x8000000a);
+ add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-off",
+ "-machine pc-i440fx-2.4 -cpu SandyBridge,",
+ "xlevel", 0x80000008);
+ add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
+ "-machine pc-i440fx-2.4 -cpu SandyBridge,+npt",
+ "xlevel", 0x80000008);
/* Test feature parsing */
add_feature_test("x86/cpuid/features/plus",
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props
2017-06-05 15:56 [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props Eduardo Habkost
@ 2017-06-05 16:07 ` Michael S. Tsirkin
2017-06-05 18:47 ` Eduardo Habkost
2017-06-05 17:36 ` Eduardo Habkost
1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2017-06-05 16:07 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel, Richard Henderson, Igor Mammedov
On Mon, Jun 05, 2017 at 12:56:55PM -0300, Eduardo Habkost wrote:
> Since the automatic cpuid-level code was introduced in commit
> c39c0edf9bb3b968ba95484465a50c7b19f4aa3a,
"target-i386: Automatically set level/xlevel/xlevel2 when needed"
> the CPU model tables just
> define the default CPUID level code (set using "min-level"). Setting
> "[x]level" forces CPUID level to a specific value and disable the
> automatic-level logic.
>
> But the PC compat code was not updated and the existing "[x]level"
> compat properties broke compatibility for people using features that
> triggered the auto-level code. To keep previous behavior, we should set
> "min-[x]level" instead of "[x]level" on compat_props.
>
> This was not a problem for most cases, because old machine-types don't
> have full-cpuid-auto-level enabled. The only common use case it broke
> was the CPUID[7] auto-level code, that was already enabled since the
> first CPUID[7] feature was introduced (in QEMU 1.4.0).
>
> This causes the regression reported at:
> https://bugzilla.redhat.com/show_bug.cgi?id=1454641
>
> Change the PC compat code to use "min-[x]level" instead of "[x]level" on
> compat_props, and add new test cases to ensure we don't break this
> again.
>
Fixes: c39c0edf9bb ("target-i386: Automatically set level/xlevel/xlevel2 when needed")
Cc: stable
> Reported-by: "Guo, Zhiyi" <zhguo@redhat.com>
> Signd-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/i386/pc.h | 42 +++++++++++++++++++++---------------------
> tests/test-x86-cpuid-compat.c | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+), 21 deletions(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index e447f5d8f4..d071c9c0e9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -566,75 +566,75 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> .value = "off",\
> },{\
> .driver = "qemu64" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(4),\
> },{\
> .driver = "kvm64" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(5),\
> },{\
> .driver = "pentium3" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(2),\
> },{\
> .driver = "n270" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(5),\
> },{\
> .driver = "Conroe" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(4),\
> },{\
> .driver = "Penryn" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(4),\
> },{\
> .driver = "Nehalem" "-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(4),\
> },{\
> .driver = "n270" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Penryn" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Conroe" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Nehalem" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Westmere" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "SandyBridge" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "IvyBridge" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Haswell" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Haswell-noTSX" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Broadwell" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = "Broadwell-noTSX" "-" TYPE_X86_CPU,\
> - .property = "xlevel",\
> + .property = "min-xlevel",\
> .value = stringify(0x8000000a),\
> },{\
> .driver = TYPE_X86_CPU,\
> @@ -860,7 +860,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> .value = stringify(2),\
> },{\
> .driver = "Conroe-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(2),\
> },{\
> .driver = "Penryn-" TYPE_X86_CPU,\
> @@ -868,7 +868,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> .value = stringify(2),\
> },{\
> .driver = "Penryn-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(2),\
> },{\
> .driver = "Nehalem-" TYPE_X86_CPU,\
> @@ -876,7 +876,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> .value = stringify(2),\
> },{\
> .driver = "Nehalem-" TYPE_X86_CPU,\
> - .property = "level",\
> + .property = "min-level",\
> .value = stringify(2),\
> },{\
> .driver = "virtio-net-pci",\
> diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
> index 6c71e46391..652216f531 100644
> --- a/tests/test-x86-cpuid-compat.c
> +++ b/tests/test-x86-cpuid-compat.c
> @@ -313,6 +313,44 @@ int main(int argc, char **argv)
> add_cpuid_test("x86/cpuid/auto-xlevel2/pc-2.7",
> "-machine pc-i440fx-2.7 -cpu 486,+xstore",
> "xlevel2", 0);
> + /*
> + * QEMU 1.4.0 had auto-level enabled for CPUID[7], already,
> + * and the compat code that sets default level shouldn't
> + * disable the auto-level=7 code:
> + */
> + add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-1.4/off",
> + "-machine pc-i440fx-1.4 -cpu Nehalem",
> + "level", 2);
> + add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-1.5/on",
> + "-machine pc-i440fx-1.4 -cpu Nehalem,+smap",
> + "level", 7);
> + add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/off",
> + "-machine pc-i440fx-2.3 -cpu Penryn",
> + "level", 4);
> + add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/on",
> + "-machine pc-i440fx-2.3 -cpu Penryn,+erms",
> + "level", 7);
> + add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/off",
> + "-machine pc-i440fx-2.9 -cpu Conroe",
> + "level", 10);
> + add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/on",
> + "-machine pc-i440fx-2.9 -cpu Conroe,+erms",
> + "level", 10);
> +
> + /*
> + * xlevel don't
don't->doesn't
> have any feature that triggers auto-level
> + * code on old machine-types. Just check if the compat code
if->that
> + * is working correctly:
> + */
> + add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.3",
> + "-machine pc-i440fx-2.3 -cpu SandyBridge",
> + "xlevel", 0x8000000a);
> + add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-off",
> + "-machine pc-i440fx-2.4 -cpu SandyBridge,",
> + "xlevel", 0x80000008);
> + add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
> + "-machine pc-i440fx-2.4 -cpu SandyBridge,+npt",
> + "xlevel", 0x80000008);
>
> /* Test feature parsing */
> add_feature_test("x86/cpuid/features/plus",
With above tweaks:
Acked-by: Michael S. Tsirkin <mst@redhat.com>
feel free to merge this through your tree.
> --
> 2.11.0.259.g40922b1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props
2017-06-05 15:56 [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props Eduardo Habkost
2017-06-05 16:07 ` Michael S. Tsirkin
@ 2017-06-05 17:36 ` Eduardo Habkost
1 sibling, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2017-06-05 17:36 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel; +Cc: Richard Henderson, Igor Mammedov
On Mon, Jun 05, 2017 at 12:56:55PM -0300, Eduardo Habkost wrote:
> Since the automatic cpuid-level code was introduced in commit
> c39c0edf9bb3b968ba95484465a50c7b19f4aa3a, the CPU model tables just
> define the default CPUID level code (set using "min-level"). Setting
> "[x]level" forces CPUID level to a specific value and disable the
> automatic-level logic.
>
> But the PC compat code was not updated and the existing "[x]level"
> compat properties broke compatibility for people using features that
> triggered the auto-level code. To keep previous behavior, we should set
> "min-[x]level" instead of "[x]level" on compat_props.
>
> This was not a problem for most cases, because old machine-types don't
> have full-cpuid-auto-level enabled. The only common use case it broke
> was the CPUID[7] auto-level code, that was already enabled since the
> first CPUID[7] feature was introduced (in QEMU 1.4.0).
>
> This causes the regression reported at:
> https://bugzilla.redhat.com/show_bug.cgi?id=1454641
>
> Change the PC compat code to use "min-[x]level" instead of "[x]level" on
> compat_props, and add new test cases to ensure we don't break this
> again.
>
> Reported-by: "Guo, Zhiyi" <zhguo@redhat.com>
> Signd-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
--
Eduardo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props
2017-06-05 16:07 ` Michael S. Tsirkin
@ 2017-06-05 18:47 ` Eduardo Habkost
0 siblings, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2017-06-05 18:47 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Richard Henderson, Igor Mammedov
On Mon, Jun 05, 2017 at 07:07:21PM +0300, Michael S. Tsirkin wrote:
[...]
>
> With above tweaks:
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> feel free to merge this through your tree.
Thanks. Queued on my x86-next branch.
--
Eduardo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-06-05 18:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-05 15:56 [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props Eduardo Habkost
2017-06-05 16:07 ` Michael S. Tsirkin
2017-06-05 18:47 ` Eduardo Habkost
2017-06-05 17:36 ` 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.