From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Andreas Färber" <afaerber@suse.de>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH qom-next v2 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Date: Fri, 26 Jul 2013 00:05:29 +0300 [thread overview]
Message-ID: <20130725210529.GB20471@redhat.com> (raw)
In-Reply-To: <1374532568-28051-5-git-send-email-afaerber@suse.de>
On Tue, Jul 23, 2013 at 12:36:08AM +0200, Andreas Färber wrote:
> Move PCIEPort's "port" property to the new type, same for "aer_log_max".
> Move PCIESlot's "chassis" and "slot" properties to the new type.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> hw/pci-bridge/ioh3420.c | 31 ++++++-----------------
> hw/pci-bridge/xio3130_downstream.c | 31 ++++++-----------------
> hw/pci-bridge/xio3130_upstream.c | 21 ++++-----------
> hw/pci/pcie_port.c | 52 ++++++++++++++++++++++++++++++++++++++
> include/hw/pci/pcie_port.h | 14 ++++++++--
> 5 files changed, 85 insertions(+), 64 deletions(-)
>
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index 728f658..e07c7e8 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -92,9 +92,8 @@ static void ioh3420_reset(DeviceState *qdev)
>
> static int ioh3420_initfn(PCIDevice *d)
> {
> - PCIBridge *br = PCI_BRIDGE(d);
> - PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
> - PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
> + PCIEPort *p = PCIE_PORT(d);
> + PCIESlot *s = PCIE_SLOT(d);
> int rc;
>
> rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> @@ -148,9 +147,7 @@ err_bridge:
>
> static void ioh3420_exitfn(PCIDevice *d)
> {
> - PCIBridge *br = PCI_BRIDGE(d);
> - PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
> - PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
> + PCIESlot *s = PCIE_SLOT(d);
>
> 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);
>
> - return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
> + return PCIE_SLOT(d);
> }
>
> static const VMStateDescription vmstate_ioh3420 = {
> @@ -190,23 +187,13 @@ static const VMStateDescription vmstate_ioh3420 = {
> .minimum_version_id_old = 1,
> .post_load = pcie_cap_slot_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(port.br.parent_obj, PCIESlot),
> - VMSTATE_STRUCT(port.br.parent_obj.exp.aer_log, PCIESlot, 0,
> - vmstate_pcie_aer_log, PCIEAERLog),
> + VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> + VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> + PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> VMSTATE_END_OF_LIST()
> }
> };
>
> -static Property ioh3420_properties[] = {
> - DEFINE_PROP_UINT8("port", PCIESlot, port.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,
> - PCIE_AER_LOG_MAX_DEFAULT),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void ioh3420_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -223,13 +210,11 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)
> dc->desc = "Intel IOH device id 3420 PCIE Root Port";
> dc->reset = ioh3420_reset;
> dc->vmsd = &vmstate_ioh3420;
> - dc->props = ioh3420_properties;
> }
>
> static const TypeInfo ioh3420_info = {
> .name = "ioh3420",
> - .parent = TYPE_PCI_BRIDGE,
> - .instance_size = sizeof(PCIESlot),
> + .parent = TYPE_PCIE_SLOT,
> .class_init = ioh3420_class_init,
> };
>
> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> index 9acce3f..2c84b1a 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 *qdev)
>
> static int xio3130_downstream_initfn(PCIDevice *d)
> {
> - PCIBridge *br = PCI_BRIDGE(d);
> - PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
> - PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
> + PCIEPort *p = PCIE_PORT(d);
> + PCIESlot *s = PCIE_SLOT(d);
> int rc;
>
> rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> @@ -113,9 +112,7 @@ err_bridge:
>
> static void xio3130_downstream_exitfn(PCIDevice *d)
> {
> - PCIBridge *br = PCI_BRIDGE(d);
> - PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
> - PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
> + PCIESlot *s = PCIE_SLOT(d);
>
> 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);
>
> - return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
> + return PCIE_SLOT(d);
> }
>
> static const VMStateDescription vmstate_xio3130_downstream = {
> @@ -157,23 +154,13 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> .minimum_version_id_old = 1,
> .post_load = pcie_cap_slot_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(port.br.parent_obj, PCIESlot),
> - VMSTATE_STRUCT(port.br.parent_obj.exp.aer_log, PCIESlot, 0,
> - vmstate_pcie_aer_log, PCIEAERLog),
> + VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> + VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> + PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> VMSTATE_END_OF_LIST()
> }
> };
>
> -static Property xio3130_downstream_properties[] = {
> - DEFINE_PROP_UINT8("port", PCIESlot, port.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,
> - PCIE_AER_LOG_MAX_DEFAULT),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void xio3130_downstream_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -190,13 +177,11 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)
> dc->desc = "TI X3130 Downstream Port of PCI Express Switch";
> dc->reset = xio3130_downstream_reset;
> dc->vmsd = &vmstate_xio3130_downstream;
> - dc->props = xio3130_downstream_properties;
> }
>
> static const TypeInfo xio3130_downstream_info = {
> .name = "xio3130-downstream",
> - .parent = TYPE_PCI_BRIDGE,
> - .instance_size = sizeof(PCIESlot),
> + .parent = TYPE_PCIE_SLOT,
> .class_init = xio3130_downstream_class_init,
> };
>
> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> index 0bc1d05..82add15 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)
>
> static int xio3130_upstream_initfn(PCIDevice *d)
> {
> - PCIBridge *br = PCI_BRIDGE(d);
> - PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
> + PCIEPort *p = PCIE_PORT(d);
> int rc;
>
> rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> @@ -125,7 +124,7 @@ PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
> qdev_prop_set_uint8(qdev, "port", port);
> qdev_init_nofail(qdev);
>
> - return DO_UPCAST(PCIEPort, br, br);
> + return PCIE_PORT(d);
> }
>
> static const VMStateDescription vmstate_xio3130_upstream = {
> @@ -134,21 +133,13 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(br.parent_obj, PCIEPort),
> - VMSTATE_STRUCT(br.parent_obj.exp.aer_log, PCIEPort, 0,
> + VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> + VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> vmstate_pcie_aer_log, PCIEAERLog),
So this horror is due to insistance of calling
the first structure in each struct "parent_obj".
I never understood why it's a good idea.
> VMSTATE_END_OF_LIST()
> }
> };
>
> -static Property xio3130_upstream_properties[] = {
> - DEFINE_PROP_UINT8("port", PCIEPort, port, 0),
> - DEFINE_PROP_UINT16("aer_log_max", PCIEPort,
> - br.parent_obj.exp.aer_log.log_max,
> - PCIE_AER_LOG_MAX_DEFAULT),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void xio3130_upstream_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -165,13 +156,11 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)
> dc->desc = "TI X3130 Upstream Port of PCI Express Switch";
> dc->reset = xio3130_upstream_reset;
> dc->vmsd = &vmstate_xio3130_upstream;
> - dc->props = xio3130_upstream_properties;
> }
>
> static const TypeInfo xio3130_upstream_info = {
> .name = "x3130-upstream",
> - .parent = TYPE_PCI_BRIDGE,
> - .instance_size = sizeof(PCIEPort),
> + .parent = TYPE_PCIE_PORT,
> .class_init = xio3130_upstream_class_init,
> };
>
> diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> index 91b53a0..2adb030 100644
> --- a/hw/pci/pcie_port.c
> +++ b/hw/pci/pcie_port.c
> @@ -116,3 +116,55 @@ void pcie_chassis_del_slot(PCIESlot *s)
> {
> QLIST_REMOVE(s, next);
> }
> +
> +static Property pcie_port_props[] = {
> + DEFINE_PROP_UINT8("port", PCIEPort, port, 0),
> + DEFINE_PROP_UINT16("aer_log_max", PCIEPort,
> + parent_obj.parent_obj.exp.aer_log.log_max,
> + PCIE_AER_LOG_MAX_DEFAULT),
> + DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void pcie_port_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + dc->props = pcie_port_props;
> +}
> +
> +static const TypeInfo pcie_port_type_info = {
> + .name = TYPE_PCIE_PORT,
> + .parent = TYPE_PCI_BRIDGE,
> + .instance_size = sizeof(PCIEPort),
> + .abstract = true,
> + .class_init = pcie_port_class_init,
> +};
> +
> +static Property pcie_slot_props[] = {
> + DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
> + DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
> + DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void pcie_slot_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + dc->props = pcie_slot_props;
> +}
> +
> +static const TypeInfo pcie_slot_type_info = {
> + .name = TYPE_PCIE_SLOT,
> + .parent = TYPE_PCIE_PORT,
> + .instance_size = sizeof(PCIESlot),
> + .abstract = true,
> + .class_init = pcie_slot_class_init,
> +};
> +
> +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"
>
> +#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 >*/
>
> /* pci express switch port */
> uint8_t port;
> @@ -33,8 +38,13 @@ struct PCIEPort {
>
> void pcie_port_init_reg(PCIDevice *d);
>
> +#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 >*/
>
> /* pci express switch port with slot */
> uint8_t chassis;
I don't like how everthing is called parent_obj instead of
what it really is, but since this appears to be in vogue
at the moment
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> --
> 1.8.1.4
next prev parent reply other threads:[~2013-07-25 21:29 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-22 22:36 [Qemu-devel] [PATCH qom-next v2 0/4] QOM'ification of pci-bridge types Andreas Färber
2013-07-22 22:36 ` [Qemu-devel] [PATCH qom-next v2 1/4] pci-bridge: Turn PCIBridge into abstract QOM type Andreas Färber
2013-07-25 20:15 ` Don Koch
2013-07-25 21:08 ` Michael S. Tsirkin
2013-07-27 0:37 ` Andreas Färber
2013-07-22 22:36 ` [Qemu-devel] [PATCH qom-next v2 2/4] pci-bridge-dev: QOM parent field cleanup Andreas Färber
2013-07-25 20:15 ` Don Koch
2013-07-22 22:36 ` [Qemu-devel] [PATCH qom-next v2 3/4] pci-bridge/i82801b11: Rename parent field Andreas Färber
2013-07-25 20:15 ` Don Koch
2013-07-22 22:36 ` [Qemu-devel] [PATCH qom-next v2 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types Andreas Färber
2013-07-25 20:15 ` Don Koch
2013-07-25 21:05 ` Michael S. Tsirkin [this message]
2013-07-28 11:50 ` [Qemu-devel] [PATCH qom-next v2 0/4] QOM'ification of pci-bridge types Andreas Färber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130725210529.GB20471@redhat.com \
--to=mst@redhat.com \
--cc=afaerber@suse.de \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.