From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL Date: Thu, 15 Oct 2009 18:33:58 +0900 Message-ID: <4AD6EC86.6060008@redhat.com> References: <20091013160647.27006.58598.stgit@mchn012c.ww002.siemens.net> <20091013160648.27006.25850.stgit@mchn012c.ww002.siemens.net> <4AD6D69E.3030408@redhat.com> <4AD6E9D4.9050109@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "kvm@vger.kernel.org" To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:49544 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934954AbZJOJe3 (ORCPT ); Thu, 15 Oct 2009 05:34:29 -0400 In-Reply-To: <4AD6E9D4.9050109@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/15/2009 06:22 PM, Jan Kiszka wrote: >> Better to split sse from fpu since we already know it is about to be >> replaced. >> > The idea is to reuse the existing state structures, including struct > kvm_fpu. This allows to provide the avoid substates for all archs and > simplifies the migration (see my qemu conversion patch). I think, once > we need support for new/wider registers in x86, we can introduce new > KVM_X86_VCPU_STATE_FPU_WHATEVER substates that are able to replace the > old one. > Makes sense, especially if we keep the list instead of the structure. >>> + >>> +struct kvm_vcpu_substate { >>> + __u32 type; >>> + __u32 pad; >>> + __s64 offset; >>> +}; >>> + >>> +#define KVM_MAX_VCPU_SUBSTATES 64 >>> + >>> +struct kvm_vcpu_state { >>> + __u32 nsubstates; /* number of elements in substates */ >>> + __u32 nprocessed; /* return value: successfully processed substates */ >>> + struct kvm_vcpu_substate substates[0]; >>> +}; >>> + >>> >>> >> Wouldn't having an ordinary struct with lots of reserved space be >> simpler? If we add a bitmask, we can even selectively get/set the >> fields we want (important if new state extends old state: avx vs sse). >> > Simpler - hmm, maybe. But also less flexible. This would establish a > static order inside this constantly growing mega struct. And a user only > interested in something small at its end would still have to allocate > memory for the whole thing (maybe megabytes in the future, who knows?). > And this mega struct will always carry all the legacy substates, even if > they aren't used anymore in practice. > I hope cpu state doesn't grow into megabytes, or we'll have problems live migrating them. But I see your point. The initial split assumed userspace would be interested in optimizing access (we used to have many more exits, and really old versions relied on qemu for emulation), that turned out not to be the case, but it's better to keep this capability for other possible userspaces. So let's go ahead with the list. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.