(CCing Paolo, as he was involved in the previous discussion about TCG vs KVM CPU models)
On Thu, Sep 19, 2013 at 01:50:58PM +0200, Andreas Färber wrote: > Hi, > > Am 18.09.2013 22:39, schrieb Eduardo Habkost: > > Hi, > > > > I would like to get your opinion on this: > > > > Currently we have x2apic enabled only on SandyBridge and Haswell CPU > > models because we try to keep the CPU models closer to real CPUs. > > However, x2apic improves performance by reducing the overhead of APIC > > accesses, and KVM can emulate it independently of host CPU support for > > x2apic. This feature is present on KVM for 4 years, already (since > > v2.6.32). There's no reason for people to not have x2apic enabled when > > running KVM. > > > > So, my question is: should we break the "try to be close to real CPUs" > > rule and enable x2apic by default on most (or all) CPU models? I believe > > it is a reasonable thing to do. > > I disagree, since this would also affect TCG. I would prefer to add > x2apic only to models that really have it and would be open to generally > enabling it for kvm_enabled() in instance_init/registration (so that > users can disable it via ,-x2apic or soon QMP). This won't affect TCG because features not supported by TCG are removed automatically, and "enforce" doesn't even work on TCG mode (yet). I believe we agreed that we don't want to make the semantics of CPU model names change depending if KVM or TCG are enabled[1], so I am trying to avoid making the default depend on kvm_enabled(). [1] http://marc.info/?l=qemu-devel&m=136983789405010&w=2 > > As always, software might make weird assumptions about effects of a > present CPUID bit, but I trust you'll do some more testing before > submitting a non-RFC patch. :) FWIW, on RHEL-6 x2apic is already set on Conroe/Penryn/Nehalem/Westmere/Opteron_G[1234], since RHEL-6.0. So at least on those CPU models, the presence of the x2apic flag already got a reasonable amount of testing. > > Regards, > Andreas > > > > > Also: if we do it, should we do it for all CPU models on > > target-i386/cpu.c, or just a subset of them? (maybe the more recent > > ones?) > > > > (The patch below touches only Conroe, Penryn, Nehalem, and Westmere, and > > it lacks machine-type compatibility code. But I am planning to submit a > > patch that changes all CPU models to include x2apic by default.) > > > > --- > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 9abb73f..f76c34b 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -791,7 +791,7 @@ static x86_def_t builtin_x86_defs[] = { > > CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE | > > CPUID_DE | CPUID_FP87, > > .features[FEAT_1_ECX] = > > - CPUID_EXT_SSSE3 | CPUID_EXT_SSE3, > > + CPUID_EXT_SSSE3 | CPUID_EXT_SSE3 | CPUID_EXT_X2APIC, > > .features[FEAT_8000_0001_EDX] = > > CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL, > > .features[FEAT_8000_0001_ECX] = > > @@ -814,7 +814,7 @@ static x86_def_t builtin_x86_defs[] = { > > CPUID_DE | CPUID_FP87, > > .features[FEAT_1_ECX] = > > CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | > > - CPUID_EXT_SSE3, > > + CPUID_EXT_SSE3 | CPUID_EXT_X2APIC, > > .features[FEAT_8000_0001_EDX] = > > CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL, > > .features[FEAT_8000_0001_ECX] = > > @@ -837,7 +837,8 @@ static x86_def_t builtin_x86_defs[] = { > > CPUID_DE | CPUID_FP87, > > .features[FEAT_1_ECX] = > > CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 | > > - CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE3, > > + CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE3 | > > + CPUID_EXT_X2APIC, > > .features[FEAT_8000_0001_EDX] = > > CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, > > .features[FEAT_8000_0001_ECX] = > > @@ -861,7 +862,7 @@ static x86_def_t builtin_x86_defs[] = { > > .features[FEAT_1_ECX] = > > CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | > > CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | > > - CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3, > > + CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 | CPUID_EXT_X2APIC, > > .features[FEAT_8000_0001_EDX] = > > CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, > > .features[FEAT_8000_0001_ECX] = > > > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- Eduardo
