From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=43486 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Phgnd-00045O-3W for qemu-devel@nongnu.org; Tue, 25 Jan 2011 06:11:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PhgnY-0006c1-F6 for qemu-devel@nongnu.org; Tue, 25 Jan 2011 06:11:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:11257) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PhgnY-0006bm-8H for qemu-devel@nongnu.org; Tue, 25 Jan 2011 06:10:56 -0500 Message-ID: <4D3EAFB0.9080302@redhat.com> Date: Tue, 25 Jan 2011 13:10:40 +0200 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state References: <4D2B6CB5.9050602@codemonkey.ws> <4D2B74D8.4080309@web.de> <4D2B8662.9060909@web.de> <4D2C60FB.7030009@linux.vnet.ibm.com> <4D2D80ED.8030405@redhat.com> <4D2D82EE.20002@siemens.com> <4D35A39A.8000801@siemens.com> <4D35ABF8.9050700@linux.vnet.ibm.com> <4D35B521.3090601@siemens.com> <4D35B6DD.1020005@linux.vnet.ibm.com> <4D3717E7.3010105@linux.vnet.ibm.com> <4D38017D.2020401@siemens.com> <4D38A7A5.7090402@web.de> In-Reply-To: <4D38A7A5.7090402@web.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: "kvm@vger.kernel.org" , Glauber Costa , Marcelo Tosatti , Markus Armbruster , "qemu-devel@nongnu.org" , Blue Swirl , Anthony Liguori On 01/20/2011 11:22 PM, Jan Kiszka wrote: > On 2011-01-20 20:27, Blue Swirl wrote: > > On Thu, Jan 20, 2011 at 9:33 AM, Jan Kiszka wrote: > >> On 2011-01-19 20:32, Blue Swirl wrote: > >>> On Wed, Jan 19, 2011 at 4:57 PM, Anthony Liguori > >>> wrote: > >>>> On 01/19/2011 07:15 AM, Markus Armbruster wrote: > >>>>> > >>>>> So they interact with KVM (need kvm_state), and they interact with the > >>>>> emulated PCI bus. Could you elaborate on the fundamental difference > >>>>> between the two interactions that makes you choose the (hypothetical) > >>>>> KVM bus over the PCI bus as device parent? > >>>>> > >>>> > >>>> It's almost arbitrary, but I would say it's the direction that I/Os flow. > >>>> > >>>> But if the underlying observation is that the device tree is not really a > >>>> tree, you're 100% correct. This is part of why a factory interface that > >>>> just takes a parent bus is too simplistic. > >>>> > >>>> I think we ought to introduce a -pci-device option that is specifically for > >>>> creating PCI devices that doesn't require a parent bus argument but provides > >>>> a way to specify stable addressing (for instancing, using a linear index). > >>> > >>> I think kvm_state should not be a property of any device or bus. It > >>> should be split to more logical pieces. > >>> > >>> Some parts of it could remain in CPUState, because they are associated > >>> with a VCPU. > >>> > >>> Also, for example irqfd could be considered to be similar object to > >>> char or block devices provided by QEMU to devices. Would it make sense > >>> to introduce new host types for passing parts of kvm_state to devices? > >>> > >>> I'd also make coalesced MMIO stuff part of memory object. We are not > >>> passing any state references when using cpu_physical_memory_rw(), but > >>> that could be changed. > >> > >> There are currently no VCPU-specific bits remaining in kvm_state. > > > > I think fields vcpu_events, robust_singlestep, debugregs, > > kvm_sw_breakpoints, xsave, xcrs belong to CPUX86State. They may be the > > same for all VCPUs but still they are sort of CPU properties. I'm not > > sure about fd field. > > They are all properties of the currently loaded KVM subsystem in the > host kernel. They can't change while KVM's root fd is opened. > Replicating this static information into each and every VCPU state would > be crazy. Perhaps they should be renamed to have_xsave or features.xsave, and be made bools, to improve readability. -- error compiling committee.c: too many arguments to function