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). 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. :) 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