From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38103 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PbUS8-0002x0-Ot for qemu-devel@nongnu.org; Sat, 08 Jan 2011 03:47:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PbUS7-000616-E1 for qemu-devel@nongnu.org; Sat, 08 Jan 2011 03:47:12 -0500 Received: from fmmailgate02.web.de ([217.72.192.227]:39628) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PbUS7-00060t-1I for qemu-devel@nongnu.org; Sat, 08 Jan 2011 03:47:11 -0500 Message-ID: <4D282489.90506@web.de> Date: Sat, 08 Jan 2011 09:47:05 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <4D2616D6.4080309@linux.vnet.ibm.com> <4D26D6CF.5070405@web.de> <4D27A16F.9030809@linux.vnet.ibm.com> In-Reply-To: <4D27A16F.9030809@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig054043DF086E725E56F023A1" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Marcelo Tosatti , qemu-devel@nongnu.org, kvm@vger.kernel.org, Alexander Graf This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig054043DF086E725E56F023A1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Am 08.01.2011 00:27, Anthony Liguori wrote: > On 01/07/2011 03:03 AM, Jan Kiszka wrote: >> Am 06.01.2011 20:24, Anthony Liguori wrote: >> =20 >>> On 01/06/2011 11:56 AM, Marcelo Tosatti wrote: >>> =20 >>>> From: Jan Kiszka >>>> >>>> QEMU supports only one VM, so there is only one kvm_state per proces= s, >>>> and we gain nothing passing a reference to it around. Eliminate any >>>> need >>>> to refer to it outside of kvm-all.c. >>>> >>>> Signed-off-by: Jan Kiszka >>>> CC: Alexander Graf >>>> Signed-off-by: Marcelo Tosatti >>>> >>>> =20 >>> I think this is a big mistake. >>> =20 >> Obviously, I don't share your concerns. :) >> >> =20 >>> Having to manage kvm_state keeps the abstraction lines well defined. >>> =20 >> How does it help? >> >> =20 >>> Otherwise, it's far too easy for portions of code to call into KVM >>> functions that really shouldn't. >>> =20 >> I can't imagine we gain anything from requiring kvm_check_extension >> callers to hold a kvm_state "capability". Yes, it's now much easier to= >> call kvm_[vm_]ioctl, but that's the key point of this change: >> >> So far we primarily complicated the internal interface between generic= >> and arch-dependent kvm parts by requiring kvm_state joggling. But >> external users already find interfaces without this restriction >> (kvm_log_*, kvm_ioeventfd_*, ...). That's because it's at least >> complicated to _cleanly_ pass kvm_state references to all users that >> need it - e.g. sysbus devices like kvmclock or upcoming in-kernel >> irqchips. >> =20 >=20 > I think you're basically making my point for me. >=20 > ioeventfd is a broken interface. It shouldn't be a VM ioctl but rather= > a VCPU ioctl because PIO events are dispatched on a per-VCPU basis. OK, but I don't want to argue about the ioeventfd API. So let's put this case aside. :) >=20 > kvm_state is available as part of CPU state so it's quite easy to get a= t > if these interfaces just took a CPUState argument (and they should). My point is definitely NOT about cpu-bound devices. That case is clear and is not touched at all by this patch. My point is about devices that have clear system scope like kvmclock, ioapic, pit, pic, whatever-the-future-will-bring. And about KVM services that have global scope like capability checks and other feature explorations or VM configurations done by the KVM arch code. You still didn't explain what we gain in these concrete scenarios by handing the technically redundant abstraction kvm_state around, especially _inside_ the KVM core. Jan --------------enig054043DF086E725E56F023A1 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/ iEYEARECAAYFAk0oJIwACgkQitSsb3rl5xSdTACg5W9rc8KUI+vldCTyZVuMXEo7 5pIAn0gv0jNewVeGXXtfPBq+D6nVDrPh =ZnSV -----END PGP SIGNATURE----- --------------enig054043DF086E725E56F023A1--