From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=42432 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PhUPS-0006Cu-Hh for qemu-devel@nongnu.org; Mon, 24 Jan 2011 16:57:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PhUPQ-0008Ek-8G for qemu-devel@nongnu.org; Mon, 24 Jan 2011 16:57:13 -0500 Received: from fmmailgate02.web.de ([217.72.192.227]:51325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PhUPP-0008EO-Q0 for qemu-devel@nongnu.org; Mon, 24 Jan 2011 16:57:12 -0500 Message-ID: <4D3DF5B2.6030405@web.de> Date: Mon, 24 Jan 2011 22:57:06 +0100 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state References: <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> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig21C30CF70FF987FE1A6642FB" Sender: jan.kiszka@web.de List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: "kvm@vger.kernel.org" , Glauber Costa , Marcelo Tosatti , Markus Armbruster , "qemu-devel@nongnu.org" , Anthony Liguori , Gerd Hoffmann , Avi Kivity This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig21C30CF70FF987FE1A6642FB Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2011-01-24 22:35, Blue Swirl wrote: > On Mon, Jan 24, 2011 at 2:08 PM, Jan Kiszka wr= ote: >> On 2011-01-21 19:49, Blue Swirl wrote: >>>>> I'd add fourth possible class: >>>>> - 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 devic= e >>>> upfront to the dynamic state of a machine, thus class 2. The point i= s, >>>> (potentially) every device of that machine requires access to it, ju= st >>>> like (indirectly, via the KVM core services) to some KVM VM state bi= ts. >>> >>> 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: >> >> - vmfd: file descriptor required for any KVM request that has VM scop= e >> (in-kernel device creation, device state synchronizations, IRQ >> routing etc.) >=20 > I'd say VM state. Good. That's +1 for introducing and distributing it. >=20 >> - irqchip_in_kernel: VM uses in-kernel irqchip acceleration >> (some devices will have to adjust their behavior depending on this) >=20 > Since QEMU version is useless, I peeked at qemu-kvm version. >=20 > There are a lot of lines like: > if (kvm_enabled() && !kvm_irqchip_in_kernel()) > kvm_just_do_it(); >=20 > Perhaps these would be cleaner with stub functions. Probably. I guess there is quite some room left for cleanups in this area= =2E >=20 > 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. Not all devices are only instantiated by the machine init code. Even if we are lucky that all those we need on x86 are created that way, we shouldn't rely on this for future use case, including other KVM archs. >=20 > 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(). >=20 Yes, callbacks are the way to go long term. Here we could also define one for VCPU interrupt handling and set it according to the VCPU mode. > I didn't look at kvm*.c, qemu-kvm*.c or stuff in kvm/. >=20 > 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. I still don't see how we can distribute the information to all interested devices. It's basically the same issue as with current kvm_sta= te. Jan --------------enig21C30CF70FF987FE1A6642FB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk099bUACgkQitSsb3rl5xRklQCaAmotHSl4rrHcMfUaD9xpQ4e5 1TQAoN65GQ0vgz+6hXlPFCixy+gU8fUN =tSsD -----END PGP SIGNATURE----- --------------enig21C30CF70FF987FE1A6642FB--