From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60186) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UInWl-00017c-1m for qemu-devel@nongnu.org; Thu, 21 Mar 2013 18:00:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UInWh-0007w7-SV for qemu-devel@nongnu.org; Thu, 21 Mar 2013 18:00:02 -0400 Date: Thu, 21 Mar 2013 16:59:48 -0500 From: Scott Wood In-Reply-To: <9F6E8CFD-3DDF-4B9E-A741-20E7FE7BC856@suse.de> (from agraf@suse.de on Thu Mar 21 16:29:02 2013) Message-ID: <1363903188.31522.25@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC ppc-next PATCH 6/6] kvm/openpic: in-kernel mpic support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: "" , "" On 03/21/2013 04:29:02 PM, Alexander Graf wrote: >=20 >=20 > Am 21.03.2013 um 21:50 schrieb Scott Wood : >=20 > > On 03/21/2013 03:41:19 AM, Alexander Graf wrote: > >> Can't all the stuff above here just simply go into the qdev init =20 > function? > > > > Not if you want platform code to be able to fall back to a QEMU =20 > mpic if an in-kernel mpic is unavailable. >=20 > Do we want that? We used to have a default like that in qemu-kvm back =20 > in the day. That was very confusing, as people started to report that =20 > their VMs turned out to be really slow. >=20 > I think we should not have fallback code. It makes things easier and =20 > more obvious. The default should just depend on the host's =20 > capabilities. I don't follow. What is the difference between "falling back" and =20 "depending on the host's capabilities"? Either we can create an =20 in-kernel MPIC or we can't. We could use KVM_CREATE_DEVICE_TEST to see =20 if the device type is supported separately from actually creating it, =20 but I don't see what that would accomplish other than adding more code. > >> > /* MPIC */ > >> > mpic =3D g_new(qemu_irq, 256); > >> > - dev =3D qdev_create(NULL, "openpic"); > >> > - qdev_prop_set_uint32(dev, "nb_cpus", smp_cpus); > >> > - qdev_prop_set_uint32(dev, "model", params->mpic_version); > >> > + > >> > + if (kvm_irqchip_wanted()) { > >> > + dev =3D kvm_openpic_create(NULL, params->mpic_version); > >> This really should be just a > >> dev =3D qdev_create(NULL, kvm_irqchip_wanted() ? "kvm-openpic" : =20 > "openpic"); > >> The logic whether an in-kernel irqchip is available belongs into =20 > the default setting of kvm_irqchip_wanted. > > > > That is exactly what I was trying to avoid by introducing =20 > kvm_irqchip_wanted. We're no longer testing some vague generic =20 > irqchip capability, but the presence of a specific type of device =20 > (and version thereof). How would the code that sets =20 > kvm_irqchip_wanted know what to test for? >=20 > Then move the default code into the board file and check for the =20 > in-kernel mpic cap. I'm not quite sure what you mean by "the default code" -- if you mean =20 the part that makes the decision whether to fall back or error out, =20 that's already in board code. -Scott=