| Message ID | 1465784487-23482-1-git-send-email-chao.p.peng@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/06/2016 04:21, Chao Peng wrote: > KVM_GET_SUPPORTED_CPUID ioctl is called frequently when initializing > CPU. Depends on CPU features and CPU count, the number of calls can be > extremely high which slows down QEMU booting significantly. In our > testing, we saw 5922 calls with switches: > > -cpu SandyBridge -smp 6,sockets=6,cores=1,threads=1 > > This ioctl takes more than 100ms, which is almost half of the total > QEMU startup time. > > While for most cases the data returned from two different invocations > are not changed, that means, we can cache the data to avoid trapping > into kernel for the second time. To make sure the cache safe one > assumption is desirable: the ioctl is stateless. This is not true > however, at least for some CPUID leaves. Which are the CPUID leaves for which KVM_GET_SUPPORTED_CPUID is not stateless? I cannot find any. > The good part is even the ioctl is not fully stateless, we can still > cache the return value if we know the data is unchanged for the leaves > we are interested in. Actually this should be true for most invocations > and looks all the places in current code hold true. > > A non-cached version can be introduced if refresh is required in the > future. [...] > > +static Notifier kvm_exit_notifier; > +static void kvm_arch_destroy(Notifier *n, void *unused) > +{ > + g_free(cpuid_cache); > +} > + > int kvm_arch_init(MachineState *ms, KVMState *s) > { > uint64_t identity_base = 0xfffbc000; > @@ -1165,6 +1176,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > smram_machine_done.notify = register_smram_listener; > qemu_add_machine_init_done_notifier(&smram_machine_done); > } > + > + kvm_exit_notifier.notify = kvm_arch_destroy; > + qemu_add_exit_notifier(&kvm_exit_notifier); > return 0; This part is unnecessary; the OS takes care of freeing the heap on exit. Thanks, Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 13, 2016 at 12:02:41PM +0200, Paolo Bonzini wrote: > > > On 13/06/2016 04:21, Chao Peng wrote: > > KVM_GET_SUPPORTED_CPUID ioctl is called frequently when initializing > > CPU. Depends on CPU features and CPU count, the number of calls can be > > extremely high which slows down QEMU booting significantly. In our > > testing, we saw 5922 calls with switches: > > > > -cpu SandyBridge -smp 6,sockets=6,cores=1,threads=1 > > > > This ioctl takes more than 100ms, which is almost half of the total > > QEMU startup time. > > > > While for most cases the data returned from two different invocations > > are not changed, that means, we can cache the data to avoid trapping > > into kernel for the second time. To make sure the cache safe one > > assumption is desirable: the ioctl is stateless. This is not true > > however, at least for some CPUID leaves. > > Which are the CPUID leaves for which KVM_GET_SUPPORTED_CPUID is not > stateless? I cannot find any. I have though leaf 0xd, sub leaf 1 is not stateless, as the size of xsave buffer(EBX) is based on XCR0 | IA32_XSS. But after looking KVM code more carefully, seems I was wrong. The code calculates EBX with the host xcr0 but not guest xcr0, nor guest IA32_XSS (not sure if this is the correct behavior), so it can always returns constant data on a certain machine. If this is not an issue, then looks we can cache it safely. > > > The good part is even the ioctl is not fully stateless, we can still > > cache the return value if we know the data is unchanged for the leaves > > we are interested in. Actually this should be true for most invocations > > and looks all the places in current code hold true. > > > > A non-cached version can be introduced if refresh is required in the > > future. > > [...] > > > > > +static Notifier kvm_exit_notifier; > > +static void kvm_arch_destroy(Notifier *n, void *unused) > > +{ > > + g_free(cpuid_cache); > > +} > > + > > int kvm_arch_init(MachineState *ms, KVMState *s) > > { > > uint64_t identity_base = 0xfffbc000; > > @@ -1165,6 +1176,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > > smram_machine_done.notify = register_smram_listener; > > qemu_add_machine_init_done_notifier(&smram_machine_done); > > } > > + > > + kvm_exit_notifier.notify = kvm_arch_destroy; > > + qemu_add_exit_notifier(&kvm_exit_notifier); > > return 0; > > > This part is unnecessary; the OS takes care of freeing the heap on exit. Make sense. Thanks. Chao -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/06/2016 07:01, Chao Peng wrote: >> > >> > Which are the CPUID leaves for which KVM_GET_SUPPORTED_CPUID is not >> > stateless? I cannot find any. > I have though leaf 0xd, sub leaf 1 is not stateless, as the size of > xsave buffer(EBX) is based on XCR0 | IA32_XSS. But after looking KVM > code more carefully, seems I was wrong. The code calculates EBX with the > host xcr0 but not guest xcr0, nor guest IA32_XSS (not sure if this is > the correct behavior), so it can always returns constant data on a > certain machine. Indeed, KVM computes the correct value at runtime, but KVM_GET_SUPPORTED_CPUID runs before there is a value for guest XCR0 or guest IA32_XSS. Thanks, I've queued the patch for QEMU 2.7. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 14, 2016 at 10:21:41AM +0200, Paolo Bonzini wrote: > > > On 14/06/2016 07:01, Chao Peng wrote: > >> > > >> > Which are the CPUID leaves for which KVM_GET_SUPPORTED_CPUID is not > >> > stateless? I cannot find any. > > I have though leaf 0xd, sub leaf 1 is not stateless, as the size of > > xsave buffer(EBX) is based on XCR0 | IA32_XSS. But after looking KVM > > code more carefully, seems I was wrong. The code calculates EBX with the > > host xcr0 but not guest xcr0, nor guest IA32_XSS (not sure if this is > > the correct behavior), so it can always returns constant data on a > > certain machine. > > Indeed, KVM computes the correct value at runtime, but > KVM_GET_SUPPORTED_CPUID runs before there is a value for guest XCR0 or > guest IA32_XSS. Yes, this is the point. > > Thanks, I've queued the patch for QEMU 2.7. Thanks :) Chao -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/target-i386/kvm.c b/target-i386/kvm.c index abf50e6..1a4d751 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -107,6 +107,8 @@ static int has_xsave; static int has_xcrs; static int has_pit_state2; +static struct kvm_cpuid2 *cpuid_cache; + int kvm_has_pit_state2(void) { return has_pit_state2; @@ -200,9 +202,14 @@ static struct kvm_cpuid2 *get_supported_cpuid(KVMState *s) { struct kvm_cpuid2 *cpuid; int max = 1; + + if (cpuid_cache != NULL) { + return cpuid_cache; + } while ((cpuid = try_get_cpuid(s, max)) == NULL) { max *= 2; } + cpuid_cache = cpuid; return cpuid; } @@ -320,8 +327,6 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, ret |= cpuid_1_edx & CPUID_EXT2_AMD_ALIASES; } - g_free(cpuid); - /* fallback for older kernels */ if ((function == KVM_CPUID_FEATURES) && !found) { ret = get_para_features(s); @@ -1090,6 +1095,12 @@ static void register_smram_listener(Notifier *n, void *unused) &smram_address_space, 1); } +static Notifier kvm_exit_notifier; +static void kvm_arch_destroy(Notifier *n, void *unused) +{ + g_free(cpuid_cache); +} + int kvm_arch_init(MachineState *ms, KVMState *s) { uint64_t identity_base = 0xfffbc000; @@ -1165,6 +1176,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s) smram_machine_done.notify = register_smram_listener; qemu_add_machine_init_done_notifier(&smram_machine_done); } + + kvm_exit_notifier.notify = kvm_arch_destroy; + qemu_add_exit_notifier(&kvm_exit_notifier); return 0; }
KVM_GET_SUPPORTED_CPUID ioctl is called frequently when initializing CPU. Depends on CPU features and CPU count, the number of calls can be extremely high which slows down QEMU booting significantly. In our testing, we saw 5922 calls with switches: -cpu SandyBridge -smp 6,sockets=6,cores=1,threads=1 This ioctl takes more than 100ms, which is almost half of the total QEMU startup time. While for most cases the data returned from two different invocations are not changed, that means, we can cache the data to avoid trapping into kernel for the second time. To make sure the cache safe one assumption is desirable: the ioctl is stateless. This is not true however, at least for some CPUID leaves. The good part is even the ioctl is not fully stateless, we can still cache the return value if we know the data is unchanged for the leaves we are interested in. Actually this should be true for most invocations and looks all the places in current code hold true. A non-cached version can be introduced if refresh is required in the future. Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com> --- target-i386/kvm.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)