On Mon, May 23, 2011 at 05:47:19PM -0400, john cooper wrote: > This patch was intended to address the replicated feature > flags in cpuid 8000_0001:edx from cpuid 0000_0001:edx. > This is due to AMD's definition where these flags are > mostly cloned in the 8000_0001:edx cpuid function. > qemu64 attempted to glue together the respective Intel > and AMD nearly disjoint features and this propagated to > the new Intel models as doing so was believed conservative > at the time. However after further soak and test lugging > around this cruft doesn't provide any value, could > conceivably confuse a guest, and has confused users trying > to maintain/add cpu definitions. This also caused issues > for libvirt attempting to track this mis-encoding. > > So we've here tossed out the AMD replicated definitions > from the Intel models, added a few replications into AMD > definitions which were missing according to AMD's latest > CPUID document, and reordered the config file flags to > follow intuitive sequential bit ordering. Also two flag > name aliases were added for clarity to Intel models. The > end result being the models definitions now conform to > their respective cpuid specifications sans x2apic which is > emulated by kvm. > > This was tested with the following combinations: > > [Conroe, Penryn, Nehalem] x [F12-64, win64, win32] -- Intel host > [Opteron_G1, Opteron_G2, Opteron_G3] x [F12-64, win64, win32] -- AMD host > > Yielding successful boots in all cases. > > Signed-off-by: john cooper <john.coo...@redhat.com> > --- > > diff --git a/sysconfigs/target/target-x86_64.conf > b/sysconfigs/target/target-x86_64.conf > index 3b85b2e..3874ff1 100644 > --- a/sysconfigs/target/target-x86_64.conf > +++ b/sysconfigs/target/target-x86_64.conf > @@ -7,9 +7,9 @@ > family = "6" > model = "15" > stepping = "3" > - feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr > tsc pse de fpu mtrr clflush mca pse36" > - feature_ecx = "sse3 ssse3 x2apic" > - extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de > fpu lm syscall nx" > + feature_edx = "sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep > apic cx8 mce pae msr tsc pse de fpu" > + feature_ecx = "x2apic ssse3 sse3" > + extfeature_edx = "i64 syscall xd" > extfeature_ecx = "lahf_lm" > xlevel = "0x8000000A" > model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)" > @@ -21,9 +21,9 @@ > family = "6" > model = "23" > stepping = "3" > - feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr > tsc pse de fpu mtrr clflush mca pse36" > - feature_ecx = "sse3 cx16 ssse3 sse4.1 x2apic" > - extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de > fpu lm syscall nx" > + feature_edx = "sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep > apic cx8 mce pae msr tsc pse de fpu" > + feature_ecx = "x2apic sse4.1 cx16 ssse3 sse3" > + extfeature_edx = "i64 syscall xd" > extfeature_ecx = "lahf_lm" > xlevel = "0x8000000A" > model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)" > @@ -35,9 +35,9 @@ > family = "6" > model = "26" > stepping = "3" > - feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr > tsc pse de fpu mtrr clflush mca pse36" > - feature_ecx = "sse3 cx16 ssse3 sse4.1 sse4.2 popcnt x2apic" > - extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de > fpu lm syscall nx" > + feature_edx = "sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep > apic cx8 mce pae msr tsc pse de fpu" > + feature_ecx = "popcnt x2apic sse4.2 sse4.1 cx16 ssse3 sse3" > + extfeature_edx = "i64 syscall xd" > extfeature_ecx = "lahf_lm" > xlevel = "0x8000000A" > model_id = "Intel Core i7 9xx (Nehalem Class Core i7)" > @@ -49,10 +49,10 @@ > family = "15" > model = "6" > stepping = "1" > - feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr > tsc pse de fpu mtrr clflush mca pse36" > - feature_ecx = "sse3 x2apic" > - extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de > fpu lm syscall nx" > -# extfeature_ecx = "" > + feature_edx = "sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep > apic cx8 mce pae msr tsc pse de fpu" > + feature_ecx = "x2apic sse3" # x2apic kvm emulated > + extfeature_edx = "lm fxsr mmx nx pse36 pat cmov mca pge mtrr syscall apic > cx8 mce pae msr tsc pse de fpu" > + extfeature_ecx = " " > xlevel = "0x80000008" > model_id = "AMD Opteron 240 (Gen 1 Class Opteron)" > > @@ -63,9 +63,9 @@ > family = "15" > model = "6" > stepping = "1" > - feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr > tsc pse de fpu mtrr clflush mca pse36" > - feature_ecx = "sse3 cx16 x2apic" > - extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de > fpu lm syscall nx rdtscp" > + feature_edx = "sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep > apic cx8 mce pae msr tsc pse de fpu" > + feature_ecx = "x2apic cx16 sse3" # x2apic kvm emulated > + extfeature_edx = "lm rdtscp fxsr mmx nx pse36 pat cmov mca pge mtrr > syscall apic cx8 mce pae msr tsc pse de fpu" > extfeature_ecx = "svm lahf_lm" > xlevel = "0x80000008" > model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)" > @@ -77,10 +77,10 @@ > family = "15" > model = "6" > stepping = "1" > - feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr > tsc pse de fpu mtrr clflush mca pse36" > - feature_ecx = "sse3 cx16 monitor popcnt x2apic" > - extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de > fpu lm syscall nx rdtscp" > - extfeature_ecx = "svm sse4a abm misalignsse lahf_lm" > + feature_edx = "sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep > apic cx8 mce pae msr tsc pse de fpu" > + feature_ecx = "popcnt x2apic cx16 monitor sse3" # x2apic kvm emulated > + extfeature_edx = "lm rdtscp fxsr mmx nx pse36 pat cmov mca pge mtrr > syscall apic cx8 mce pae msr tsc pse de fpu" > + extfeature_ecx = "misalignsse sse4a abm svm lahf_lm" > xlevel = "0x80000008" > model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)" > > diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c > index c151f12..fc72f7b 100644 > --- a/target-i386/cpuid.c > +++ b/target-i386/cpuid.c > @@ -57,9 +57,9 @@ static const char *ext2_feature_name[] = { > "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", > "mtrr", "pge", "mca", "cmov", > "pat", "pse36", NULL, NULL /* Linux mp */, > - "nx" /* Intel xd */, NULL, "mmxext", "mmx", > + "nx|xd", NULL, "mmxext", "mmx", > "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, > "rdtscp", > - NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow", > + NULL, "lm|i64", "3dnowext", "3dnow", > }; > static const char *ext3_feature_name[] = { > "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD > ExtApicSpace */,
This diff seems to have a huge, arbitrary re-ordering of existing flags, combined with a tiny number of changes. It would be really nice if the arbitrary re-ordering was removed, leaving just the functional changes clearly visible. Alternatively split the patch into 2, the first doing a no-functional-change reordering, and the second doing the actual fixes. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|