From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VbVZf-0002wj-Ui for qemu-devel@nongnu.org; Wed, 30 Oct 2013 09:12:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VbVZW-00058g-9w for qemu-devel@nongnu.org; Wed, 30 Oct 2013 09:12:39 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49289 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VbVZW-00058Q-0k for qemu-devel@nongnu.org; Wed, 30 Oct 2013 09:12:30 -0400 Message-ID: <527105B9.3010104@suse.de> Date: Wed, 30 Oct 2013 14:12:25 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1383062897-30084-1-git-send-email-armbru@redhat.com> <1383062897-30084-6-git-send-email-armbru@redhat.com> <1383126348.4734.19.camel@localhost.localdomain> <87sivjlz52.fsf@blackfin.pond.sub.org> In-Reply-To: <87sivjlz52.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Marcel Apfelbaum Cc: kwolf@redhat.com, borntraeger@de.ibm.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, peter.maydell@linaro.org Am 30.10.2013 13:30, schrieb Markus Armbruster: > Marcel Apfelbaum writes: >=20 >> On Tue, 2013-10-29 at 17:08 +0100, armbru@redhat.com wrote: >>> From: Markus Armbruster >>> >>> Many PCI host bridges consist of a sysbus device and a PCI device. >>> You need both for the thing to work. Arguably, these bridges should >>> be modelled as a single, composite devices instead of pairs of >>> seemingly independent devices you can only use together, but we're no= t >>> there, yet. >>> >>> Since the sysbus part can't be instantiated with device_add, yet, >>> permitting it with the PCI part is useless. We shouldn't offer >>> useless options to the user, so let's set >>> cannot_instantiate_with_device_add_yet for them. >>> >>> It's already set for Bonito, grackle, i440FX, and raven. Document >>> why. >>> >>> Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch, >>> pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp, >>> uni-north-internal-pci, uni-north-pci, and versatile_pci_host. >>> >>> Signed-off-by: Markus Armbruster >>> --- >>> hw/mips/gt64xxx_pci.c | 6 ++++++ >>> hw/pci-bridge/dec.c | 6 ++++++ >>> hw/pci-host/apb.c | 6 ++++++ >>> hw/pci-host/bonito.c | 6 +++++- >>> hw/pci-host/grackle.c | 6 +++++- >>> hw/pci-host/piix.c | 6 +++++- >>> hw/pci-host/ppce500.c | 5 +++++ >>> hw/pci-host/prep.c | 6 +++++- >>> hw/pci-host/q35.c | 5 +++++ >>> hw/pci-host/uninorth.c | 24 ++++++++++++++++++++++++ >>> hw/pci-host/versatile.c | 6 ++++++ >>> hw/ppc/ppc4xx_pci.c | 5 +++++ >>> hw/sh4/sh_pci.c | 6 ++++++ >>> 13 files changed, 89 insertions(+), 4 deletions(-) >>> >>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c >>> index 3da2e67..6398514 100644 >>> --- a/hw/mips/gt64xxx_pci.c >>> +++ b/hw/mips/gt64xxx_pci.c >>> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d) >>> static void gt64120_pci_class_init(ObjectClass *klass, void *data) >>> { >>> PCIDeviceClass *k =3D PCI_DEVICE_CLASS(klass); >>> + DeviceClass *dc =3D DEVICE_CLASS(klass); >>> =20 >>> k->init =3D gt64120_pci_init; >>> k->vendor_id =3D PCI_VENDOR_ID_MARVELL; >>> k->device_id =3D PCI_DEVICE_ID_MARVELL_GT6412X; >>> k->revision =3D 0x10; >>> k->class_id =3D PCI_CLASS_BRIDGE_HOST; >>> + /* >>> + * PCI-facing part of the host bridge, not usable without the >>> + * host-facing part, which can't be device_add'ed, yet. >>> + */ >>> + dc->cannot_instantiate_with_device_add_yet =3D true; >> I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST. >=20 > Correct. >=20 >> What do you think about a different approach: check class_id >> in parent class init func and set the flag according to it? >> It corresponds to your idea of changing only sysbus base class. >> Here we don't have a "natural" base class, but we can use class_id. >> What do you think? >=20 > My understanding of QOM is rather limited, so take the following with > due skepticism. >=20 > I'm afraid the parent's class_init() runs before the child's, and > therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's > class_init(). Right. > To factor common initialization code, I figure I'd have to splice in an > abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host > bridge types such as this one. Might make sense, but it's a bit more > than I bargained for in this series :) I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the controller on the PCIBus it exposes? Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg