From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41018) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dHuXP-0003wa-QC for qemu-devel@nongnu.org; Mon, 05 Jun 2017 12:07:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dHuXM-0000ta-Ir for qemu-devel@nongnu.org; Mon, 05 Jun 2017 12:07:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7627) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dHuXM-0000sp-8W for qemu-devel@nongnu.org; Mon, 05 Jun 2017 12:07:24 -0400 Date: Mon, 5 Jun 2017 19:07:21 +0300 From: "Michael S. Tsirkin" Message-ID: <20170605190434-mutt-send-email-mst@kernel.org> References: <20170605155655.19315-1-ehabkost@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170605155655.19315-1-ehabkost@redhat.com> Subject: Re: [Qemu-devel] [PATCH] pc: Use "min-[x]level" on compat_props List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, 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" > Signd-off-by: Eduardo Habkost > --- > 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 feel free to merge this through your tree. > -- > 2.11.0.259.g40922b1