From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43320) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V10BY-0004L9-8E for qemu-devel@nongnu.org; Sun, 21 Jul 2013 16:24:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V10BT-0005EC-8t for qemu-devel@nongnu.org; Sun, 21 Jul 2013 16:24:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39189) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V10BS-0005E8-Vo for qemu-devel@nongnu.org; Sun, 21 Jul 2013 16:24:47 -0400 Date: Sun, 21 Jul 2013 23:26:07 +0300 From: "Michael S. Tsirkin" Message-ID: <20130721202607.GA15187@redhat.com> References: <1374415744-6675-1-git-send-email-afaerber@suse.de> <1374415744-6675-5-git-send-email-afaerber@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1374415744-6675-5-git-send-email-afaerber@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org On Sun, Jul 21, 2013 at 04:09:04PM +0200, Andreas F=E4rber wrote: > Signed-off-by: Andreas F=E4rber > --- > hw/pci-bridge/ioh3420.c | 23 ++++++++++------------- > hw/pci-bridge/xio3130_downstream.c | 23 ++++++++++------------- > hw/pci-bridge/xio3130_upstream.c | 15 +++++++-------- > hw/pci/pcie_port.c | 22 ++++++++++++++++++++++ > include/hw/pci/pcie_port.h | 14 ++++++++++++-- > 5 files changed, 61 insertions(+), 36 deletions(-) >=20 > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c > index 728f658..83db054 100644 > --- a/hw/pci-bridge/ioh3420.c > +++ b/hw/pci-bridge/ioh3420.c > @@ -92,9 +92,8 @@ static void ioh3420_reset(DeviceState *qdev) > =20 > static int ioh3420_initfn(PCIDevice *d) > { > - PCIBridge *br =3D PCI_BRIDGE(d); > - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > - PCIESlot *s =3D DO_UPCAST(PCIESlot, port, p); > + PCIEPort *p =3D PCIE_PORT(d); > + PCIESlot *s =3D PCIE_SLOT(d); > int rc; > =20 > rc =3D pci_bridge_initfn(d, TYPE_PCIE_BUS); > @@ -148,9 +147,7 @@ err_bridge: > =20 > static void ioh3420_exitfn(PCIDevice *d) > { > - PCIBridge *br =3D PCI_BRIDGE(d); > - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > - PCIESlot *s =3D DO_UPCAST(PCIESlot, port, p); > + PCIESlot *s =3D PCIE_SLOT(d); > =20 > pcie_aer_exit(d); > pcie_chassis_del_slot(s); > @@ -180,7 +177,7 @@ PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool= multifunction, > qdev_prop_set_uint16(qdev, "slot", slot); > qdev_init_nofail(qdev); > =20 > - return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br)); > + return PCIE_SLOT(d); > } > =20 > static const VMStateDescription vmstate_ioh3420 =3D { > @@ -190,19 +187,19 @@ static const VMStateDescription vmstate_ioh3420 =3D= { > .minimum_version_id_old =3D 1, > .post_load =3D pcie_cap_slot_post_load, > .fields =3D (VMStateField[]) { > - VMSTATE_PCIE_DEVICE(port.br.parent_obj, PCIESlot), > - VMSTATE_STRUCT(port.br.parent_obj.exp.aer_log, PCIESlot, 0, > + VMSTATE_PCIE_DEVICE(parent_obj, PCIBridge), > + VMSTATE_STRUCT(exp.aer_log, PCIDevice, 0, > vmstate_pcie_aer_log, PCIEAERLog), > VMSTATE_END_OF_LIST() > } > }; > =20 > static Property ioh3420_properties[] =3D { > - DEFINE_PROP_UINT8("port", PCIESlot, port.port, 0), > + DEFINE_PROP_UINT8("port", PCIEPort, port, 0), > DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0), > DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0), > - DEFINE_PROP_UINT16("aer_log_max", PCIESlot, > - port.br.parent_obj.exp.aer_log.log_max, > + DEFINE_PROP_UINT16("aer_log_max", PCIDevice, > + exp.aer_log.log_max, > PCIE_AER_LOG_MAX_DEFAULT), > DEFINE_PROP_END_OF_LIST(), > }; This looks scary. This does a cast to different types without any checks at all. What are the advantages of this hack? > @@ -228,7 +225,7 @@ static void ioh3420_class_init(ObjectClass *klass, = void *data) > =20 > static const TypeInfo ioh3420_info =3D { > .name =3D "ioh3420", > - .parent =3D TYPE_PCI_BRIDGE, > + .parent =3D TYPE_PCIE_SLOT, > .instance_size =3D sizeof(PCIESlot), > .class_init =3D ioh3420_class_init, > }; > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130= _downstream.c > index 9acce3f..549ef26 100644 > --- a/hw/pci-bridge/xio3130_downstream.c > +++ b/hw/pci-bridge/xio3130_downstream.c > @@ -56,9 +56,8 @@ static void xio3130_downstream_reset(DeviceState *qde= v) > =20 > static int xio3130_downstream_initfn(PCIDevice *d) > { > - PCIBridge *br =3D PCI_BRIDGE(d); > - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > - PCIESlot *s =3D DO_UPCAST(PCIESlot, port, p); > + PCIEPort *p =3D PCIE_PORT(d); > + PCIESlot *s =3D PCIE_SLOT(d); > int rc; > =20 > rc =3D pci_bridge_initfn(d, TYPE_PCIE_BUS); > @@ -113,9 +112,7 @@ err_bridge: > =20 > static void xio3130_downstream_exitfn(PCIDevice *d) > { > - PCIBridge *br =3D PCI_BRIDGE(d); > - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > - PCIESlot *s =3D DO_UPCAST(PCIESlot, port, p); > + PCIESlot *s =3D PCIE_SLOT(d); > =20 > pcie_aer_exit(d); > pcie_chassis_del_slot(s); > @@ -147,7 +144,7 @@ PCIESlot *xio3130_downstream_init(PCIBus *bus, int = devfn, bool multifunction, > qdev_prop_set_uint16(qdev, "slot", slot); > qdev_init_nofail(qdev); > =20 > - return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br)); > + return PCIE_SLOT(d); > } > =20 > static const VMStateDescription vmstate_xio3130_downstream =3D { > @@ -157,19 +154,19 @@ static const VMStateDescription vmstate_xio3130_d= ownstream =3D { > .minimum_version_id_old =3D 1, > .post_load =3D pcie_cap_slot_post_load, > .fields =3D (VMStateField[]) { > - VMSTATE_PCIE_DEVICE(port.br.parent_obj, PCIESlot), > - VMSTATE_STRUCT(port.br.parent_obj.exp.aer_log, PCIESlot, 0, > + VMSTATE_PCIE_DEVICE(parent_obj, PCIBridge), > + VMSTATE_STRUCT(exp.aer_log, PCIDevice, 0, > vmstate_pcie_aer_log, PCIEAERLog), > VMSTATE_END_OF_LIST() > } > }; > =20 > static Property xio3130_downstream_properties[] =3D { > - DEFINE_PROP_UINT8("port", PCIESlot, port.port, 0), > + DEFINE_PROP_UINT8("port", PCIEPort, port, 0), > DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0), > DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0), > - DEFINE_PROP_UINT16("aer_log_max", PCIESlot, > - port.br.parent_obj.exp.aer_log.log_max, > + DEFINE_PROP_UINT16("aer_log_max", PCIDevice, > + exp.aer_log.log_max, > PCIE_AER_LOG_MAX_DEFAULT), > DEFINE_PROP_END_OF_LIST(), > }; Same question. > @@ -195,7 +192,7 @@ static void xio3130_downstream_class_init(ObjectCla= ss *klass, void *data) > =20 > static const TypeInfo xio3130_downstream_info =3D { > .name =3D "xio3130-downstream", > - .parent =3D TYPE_PCI_BRIDGE, > + .parent =3D TYPE_PCIE_SLOT, > .instance_size =3D sizeof(PCIESlot), > .class_init =3D xio3130_downstream_class_init, > }; > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_u= pstream.c > index 6bbcf98..b1ee464 100644 > --- a/hw/pci-bridge/xio3130_upstream.c > +++ b/hw/pci-bridge/xio3130_upstream.c > @@ -53,8 +53,7 @@ static void xio3130_upstream_reset(DeviceState *qdev) > =20 > static int xio3130_upstream_initfn(PCIDevice *d) > { > - PCIBridge *br =3D PCI_BRIDGE(d); > - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > + PCIEPort *p =3D PCIE_PORT(d); > int rc; > =20 > rc =3D pci_bridge_initfn(d, TYPE_PCIE_BUS); > @@ -125,7 +124,7 @@ PCIEPort *xio3130_upstream_init(PCIBus *bus, int de= vfn, bool multifunction, > qdev_prop_set_uint8(qdev, "port", port); > qdev_init_nofail(qdev); > =20 > - return DO_UPCAST(PCIEPort, br, br); > + return PCIE_PORT(d); > } > =20 > static const VMStateDescription vmstate_xio3130_upstream =3D { > @@ -134,8 +133,8 @@ static const VMStateDescription vmstate_xio3130_ups= tream =3D { > .minimum_version_id =3D 1, > .minimum_version_id_old =3D 1, > .fields =3D (VMStateField[]) { > - VMSTATE_PCIE_DEVICE(br.parent_obj, PCIEPort), > - VMSTATE_STRUCT(br.parent_obj.exp.aer_log, PCIEPort, 0, > + VMSTATE_PCIE_DEVICE(parent_obj, PCIBridge), > + VMSTATE_STRUCT(exp.aer_log, PCIDevice, 0, > vmstate_pcie_aer_log, PCIEAERLog), > VMSTATE_END_OF_LIST() > } > @@ -143,8 +142,8 @@ static const VMStateDescription vmstate_xio3130_ups= tream =3D { > =20 > static Property xio3130_upstream_properties[] =3D { > DEFINE_PROP_UINT8("port", PCIEPort, port, 0), > - DEFINE_PROP_UINT16("aer_log_max", PCIEPort, > - br.parent_obj.exp.aer_log.log_max, > + DEFINE_PROP_UINT16("aer_log_max", PCIDevice, > + exp.aer_log.log_max, > PCIE_AER_LOG_MAX_DEFAULT), > DEFINE_PROP_END_OF_LIST(), > }; Same question. > @@ -170,7 +169,7 @@ static void xio3130_upstream_class_init(ObjectClass= *klass, void *data) > =20 > static const TypeInfo xio3130_upstream_info =3D { > .name =3D "x3130-upstream", > - .parent =3D TYPE_PCI_BRIDGE, > + .parent =3D TYPE_PCIE_PORT, > .instance_size =3D sizeof(PCIEPort), > .class_init =3D xio3130_upstream_class_init, > }; > diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c > index 91b53a0..a5333a7 100644 > --- a/hw/pci/pcie_port.c > +++ b/hw/pci/pcie_port.c > @@ -116,3 +116,25 @@ void pcie_chassis_del_slot(PCIESlot *s) > { > QLIST_REMOVE(s, next); > } > + > +static const TypeInfo pcie_port_type_info =3D { > + .name =3D TYPE_PCIE_PORT, > + .parent =3D TYPE_PCI_BRIDGE, > + .instance_size =3D sizeof(PCIEPort), > + .abstract =3D true, > +}; > + > +static const TypeInfo pcie_slot_type_info =3D { > + .name =3D TYPE_PCIE_SLOT, > + .parent =3D TYPE_PCIE_PORT, > + .instance_size =3D sizeof(PCIESlot), > + .abstract =3D true, > +}; > + > +static void pcie_port_register_types(void) > +{ > + type_register_static(&pcie_port_type_info); > + type_register_static(&pcie_slot_type_info); > +} > + > +type_init(pcie_port_register_types) > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h > index d89aa61..e167bf7 100644 > --- a/include/hw/pci/pcie_port.h > +++ b/include/hw/pci/pcie_port.h > @@ -24,8 +24,13 @@ > #include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" > =20 > +#define TYPE_PCIE_PORT "pcie-port" > +#define PCIE_PORT(obj) OBJECT_CHECK(PCIEPort, (obj), TYPE_PCIE_PORT) > + > struct PCIEPort { > - PCIBridge br; > + /*< private >*/ > + PCIBridge parent_obj; > + /*< public >*/ > =20 > /* pci express switch port */ > uint8_t port; > @@ -33,8 +38,13 @@ struct PCIEPort { > =20 > void pcie_port_init_reg(PCIDevice *d); > =20 > +#define TYPE_PCIE_SLOT "pcie-slot" > +#define PCIE_SLOT(obj) OBJECT_CHECK(PCIESlot, (obj), TYPE_PCIE_SLOT) > + > struct PCIESlot { > - PCIEPort port; > + /*< private >*/ > + PCIEPort parent_obj; > + /*< public >*/ > =20 > /* pci express switch port with slot */ > uint8_t chassis; > --=20 > 1.8.1.4