From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=45632 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PhU4n-00052d-HT for qemu-devel@nongnu.org; Mon, 24 Jan 2011 16:36:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PhU4X-0004EU-CG for qemu-devel@nongnu.org; Mon, 24 Jan 2011 16:35:47 -0500 Received: from mail-pz0-f45.google.com ([209.85.210.45]:44764) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PhU4X-0004E0-56 for qemu-devel@nongnu.org; Mon, 24 Jan 2011 16:35:37 -0500 Received: by pzk2 with SMTP id 2so899457pzk.4 for ; Mon, 24 Jan 2011 13:35:36 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <4D3D87EB.30508@siemens.com> 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> <4D3947D2.7070802@redhat.com> <4D39C0B4.3070807@siemens.com> <4D39CDBB.1000709@siemens.com> <4D3D87EB.30508@siemens.com> From: Blue Swirl Date: Mon, 24 Jan 2011 21:35:15 +0000 Message-ID: Subject: Re: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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" , Anthony Liguori , Gerd Hoffmann , Avi Kivity On Mon, Jan 24, 2011 at 2:08 PM, Jan Kiszka wrote: > On 2011-01-21 19:49, Blue Swirl wrote: >>>> I'd add fourth possible class: >>>> =C2=A0- device, CPU and machine configuration, like nographic, >>>> win2k_install_hack, no_hpet, smp_cpus etc. Maybe also >>>> irqchip_in_kernel could fit here, though it obviously depends on a >>>> host capability too. >>> >>> I would count everything that cannot be assigned to a concrete device >>> upfront to the dynamic state of a machine, thus class 2. The point is, >>> (potentially) every device of that machine requires access to it, just >>> like (indirectly, via the KVM core services) to some KVM VM state bits. >> >> The machine class should not be a catch-all, it would be like >> QEMUState or KVMState then. Perhaps each field or variable should be >> listed and given more thought. > > Let's start with what is most urgent: > > =C2=A0- vmfd: file descriptor required for any KVM request that has VM sc= ope > =C2=A0 (in-kernel device creation, device state synchronizations, IRQ > =C2=A0 routing etc.) I'd say VM state. > =C2=A0- irqchip_in_kernel: VM uses in-kernel irqchip acceleration > =C2=A0 (some devices will have to adjust their behavior depending on this= ) Since QEMU version is useless, I peeked at qemu-kvm version. There are a lot of lines like: if (kvm_enabled() && !kvm_irqchip_in_kernel()) kvm_just_do_it(); Perhaps these would be cleaner with stub functions. The device cases are obvious: the devices need a flag, passed to them by pc.c, which combines kvm_enabled && kvm_irqchip_in_kernel(). This gets stored in device state. But exec.c case, where kvm_update_interrupt_request() is called, is more interesting. CPU init could set up function pointer to either stub/NULL or kvm_update_interrupt_request(). I didn't look at kvm*.c, qemu-kvm*.c or stuff in kvm/. So I'd eliminate kvm_irqchip_in_kernel() from outside of KVM and pc.c. The information could be stored in a MachineState, where pc.c could grab it for device and CPU setup.