QOM'ification of pci-bridge types
cover QOM'ification of pci-bridge types
Hello Michael et al.,
This series turns PCIBridge, PCIEPort and PCIESlot into abstract QOM types, so that we can use QOM casts to obtain a pointer.
Possibly this was prompted by q35's PCIe? Don't remember ATM...
Regards,
Andreas
Andreas Färber (4):
pci-bridge: Turn into abstract QOM type
pci-bridge-dev: QOM parent field cleanup pci-bridge/i82801b11: Rename parent field
pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
hw/pci-bridge/dec.c | 4 ++-- hw/pci-bridge/i82801b11.c | 10 ++++++---- hw/pci-bridge/ioh3420.c | 29 +++++++++++++-------------- hw/pci-bridge/pci_bridge_dev.c | 26 +++++++++++++++---------- hw/pci-bridge/xio3130_downstream.c | 29 +++++++++++++-------------- hw/pci-bridge/xio3130_upstream.c | 20 +++++++++---------- hw/pci-host/apb.c | 4 ++-- hw/pci/pci_bridge.c | 40 +++++++++++++++++++++++++++----------- hw/pci/pcie.c | 2 +- hw/pci/pcie_port.c | 22 +++++++++++++++++++++ include/hw/pci/pci_bus.h | 7 ++++++- include/hw/pci/pcie_port.h | 14 +++++++++++-- 12 files changed, 132 insertions(+), 75 deletions(-)
Signature · 1 line
1.8.1.4
RFC PATCH 1/4 pci-bridge: Turn into abstract QOM type
Introduce TYPE_PCI_BRIDGE as base type and use PCI_BRIDGE() casts.
--- hw/pci-bridge/dec.c | 4 ++-- hw/pci-bridge/i82801b11.c | 6 +++--- hw/pci-bridge/ioh3420.c | 18 ++++++++--------- hw/pci-bridge/pci_bridge_dev.c | 10 +++++----- hw/pci-bridge/xio3130_downstream.c | 18 ++++++++--------- hw/pci-bridge/xio3130_upstream.c | 17 ++++++++-------- hw/pci-host/apb.c | 4 ++-- hw/pci/pci_bridge.c | 40 +++++++++++++++++++++++++++----------- hw/pci/pcie.c | 2 +- include/hw/pci/pci_bus.h | 7 ++++++- 10 files changed, 75 insertions(+), 51 deletions(-)
hw/pci-bridge/dec.c +2 -2
@@ -74,7 +74,7 @@ static void dec_21154_pci_bridge_class_init(ObjectClass *klass, void *data)static const TypeInfo dec_21154_pci_bridge_info = { .name ="dec-21154-p2p-bridge" , - .parent =TYPE_PCI_DEVICE , + .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (PCIBridge), .class_init = dec_21154_pci_bridge_class_init, };
@@ -86,7 +86,7 @@ PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn) dev =pci_create_multifunction (parent_bus, devfn, false,"dec-21154-p2p-bridge" ); - br =DO_UPCAST (PCIBridge, dev, dev); + br =PCI_BRIDGE (dev);pci_bridge_map_irq (br,"DEC 21154 PCI-PCI bridge" , dec_map_irq);qdev_init_nofail (&dev->qdev);return pci_bridge_get_sec_bus (br);
hw/pci-bridge/i82801b11.c +3 -3
@@ -91,7 +91,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data)static const TypeInfo i82801b11_bridge_info = { .name ="i82801b11-bridge" , - .parent =TYPE_PCI_DEVICE , + .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (I82801b11Bridge), .class_init = i82801b11_bridge_class_init, };
@@ -107,8 +107,8 @@ PCIBus *ich9_d2pbr_init(PCIBus *bus, int devfn, int sec_bus)if (!d) {return NULL ; } - br =DO_UPCAST (PCIBridge, dev, d); - qdev = &br->dev.qdev; + br =PCI_BRIDGE (d); + qdev =DEVICE (d);snprintf (buf,sizeof (buf),"pci.%d" , sec_bus);pci_bridge_map_irq (br, buf, pci_swizzle_map_irq_fn);
hw/pci-bridge/ioh3420.c +9 -9
@@ -92,7 +92,7 @@ static void ioh3420_reset(DeviceState *qdev)static int ioh3420_initfn (PCIDevice *d) { -PCIBridge * br =DO_UPCAST (PCIBridge, dev, d); +PCIBridge *br =PCI_BRIDGE (d);PCIEPort *p =DO_UPCAST (PCIEPort, br, br);PCIESlot *s =DO_UPCAST (PCIESlot, port, p);int rc;
@@ -148,7 +148,7 @@ err_bridge:static void ioh3420_exitfn (PCIDevice *d) { -PCIBridge * br =DO_UPCAST (PCIBridge, dev, d); +PCIBridge *br =PCI_BRIDGE (d);PCIEPort *p =DO_UPCAST (PCIEPort, br, br);PCIESlot *s =DO_UPCAST (PCIESlot, port, p);
@@ -171,9 +171,9 @@ PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool multifunction,if (!d) {return NULL ; } - br =DO_UPCAST (PCIBridge, dev, d); + br =PCI_BRIDGE (d); - qdev = &br->dev.qdev; + qdev =DEVICE (d);pci_bridge_map_irq (br, bus_name, map_irq);qdev_prop_set_uint8 (qdev,"port" , port);qdev_prop_set_uint8 (qdev,"chassis" , chassis);
@@ -190,8 +190,8 @@ static const VMStateDescription vmstate_ioh3420 = { .minimum_version_id_old = 1, .post_load = pcie_cap_slot_post_load, .fields = (VMStateField[]) { -VMSTATE_PCIE_DEVICE (port.br.dev, PCIESlot), -VMSTATE_STRUCT (port.br.dev.exp.aer_log, PCIESlot,0 , +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_END_OF_LIST () }
@@ -202,8 +202,8 @@ static Property ioh3420_properties[] = {DEFINE_PROP_UINT8 ("chassis" , PCIESlot, chassis,0 ),DEFINE_PROP_UINT16 ("slot" , PCIESlot, slot,0 ),DEFINE_PROP_UINT16 ("aer_log_max" , PCIESlot, - port.br.dev.exp.aer_log.log_max, -PCIE_AER_LOG_MAX_DEFAULT ), + port.br.parent_obj.exp.aer_log.log_max, +PCIE_AER_LOG_MAX_DEFAULT ), DEFINE_PROP_END_OF_LIST(), };
@@ -228,7 +228,7 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)static const TypeInfo ioh3420_info = { .name ="ioh3420" , - .parent =TYPE_PCI_DEVICE , + .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (PCIESlot), .class_init = ioh3420_class_init, };
hw/pci-bridge/pci_bridge_dev.c +5 -5
@@ -38,7 +38,7 @@ typedef struct PCIBridgeDev PCIBridgeDev;static int pci_bridge_dev_initfn (PCIDevice *dev) { -PCIBridge *br =DO_UPCAST (PCIBridge, dev, dev); +PCIBridge *br =PCI_BRIDGE (dev);PCIBridgeDev *bridge_dev =DO_UPCAST (PCIBridgeDev, bridge, br);int err;
@@ -81,7 +81,7 @@ bridge_error:static void pci_bridge_dev_exitfn (PCIDevice *dev) { -PCIBridge *br =DO_UPCAST (PCIBridge, dev, dev); +PCIBridge *br =PCI_BRIDGE (dev);PCIBridgeDev *bridge_dev =DO_UPCAST (PCIBridgeDev, bridge, br);if (msi_present (dev)) {msi_uninit (dev);
@@ -120,8 +120,8 @@ static Property pci_bridge_dev_properties[] = {static const VMStateDescription pci_bridge_dev_vmstate = { .name ="pci_bridge" , .fields = (VMStateField []) { -VMSTATE_PCI_DEVICE (bridge.dev, PCIBridgeDev), -SHPC_VMSTATE (bridge.dev.shpc, PCIBridgeDev), +VMSTATE_PCI_DEVICE (bridge.parent_obj, PCIBridgeDev), +SHPC_VMSTATE (bridge.parent_obj.shpc, PCIBridgeDev),VMSTATE_END_OF_LIST () } };
@@ -145,7 +145,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)static const TypeInfo pci_bridge_dev_info = { .name ="pci-bridge" , - .parent =TYPE_PCI_DEVICE , + .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (PCIBridgeDev), .class_init = pci_bridge_dev_class_init, };
hw/pci-bridge/xio3130_downstream.c +9 -9
@@ -56,7 +56,7 @@ static void xio3130_downstream_reset(DeviceState *qdev)static int xio3130_downstream_initfn (PCIDevice *d) { -PCIBridge * br =DO_UPCAST (PCIBridge, dev, d); +PCIBridge *br =PCI_BRIDGE (d);PCIEPort *p =DO_UPCAST (PCIEPort, br, br);PCIESlot *s =DO_UPCAST (PCIESlot, port, p);int rc;
@@ -113,7 +113,7 @@ err_bridge:static void xio3130_downstream_exitfn (PCIDevice *d) { -PCIBridge * br =DO_UPCAST (PCIBridge, dev, d); +PCIBridge *br =PCI_BRIDGE (d);PCIEPort *p =DO_UPCAST (PCIEPort, br, br);PCIESlot *s =DO_UPCAST (PCIESlot, port, p);
@@ -138,9 +138,9 @@ PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,if (!d) {return NULL ; } - br =DO_UPCAST (PCIBridge, dev, d); + br =PCI_BRIDGE (d); - qdev = &br->dev.qdev; + qdev =DEVICE (d);pci_bridge_map_irq (br, bus_name, map_irq);qdev_prop_set_uint8 (qdev,"port" , port);qdev_prop_set_uint8 (qdev,"chassis" , chassis);
@@ -157,8 +157,8 @@ 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.dev, PCIESlot), -VMSTATE_STRUCT (port.br.dev.exp.aer_log, PCIESlot,0 , +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_END_OF_LIST () }
@@ -169,8 +169,8 @@ static Property xio3130_downstream_properties[] = {DEFINE_PROP_UINT8 ("chassis" , PCIESlot, chassis,0 ),DEFINE_PROP_UINT16 ("slot" , PCIESlot, slot,0 ),DEFINE_PROP_UINT16 ("aer_log_max" , PCIESlot, - port.br.dev.exp.aer_log.log_max, -PCIE_AER_LOG_MAX_DEFAULT ), + port.br.parent_obj.exp.aer_log.log_max, +PCIE_AER_LOG_MAX_DEFAULT ), DEFINE_PROP_END_OF_LIST(), };
@@ -195,7 +195,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_downstream_info = { .name ="xio3130-downstream" , - .parent =TYPE_PCI_DEVICE , + .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (PCIESlot), .class_init = xio3130_downstream_class_init, };
hw/pci-bridge/xio3130_upstream.c +9 -8
@@ -53,7 +53,7 @@ static void xio3130_upstream_reset(DeviceState *qdev)static int xio3130_upstream_initfn (PCIDevice *d) { -PCIBridge * br =DO_UPCAST (PCIBridge, dev, d); +PCIBridge *br =PCI_BRIDGE (d);PCIEPort *p =DO_UPCAST (PCIEPort, br, br);int rc;
@@ -118,9 +118,9 @@ PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,if (!d) {return NULL ; } - br =DO_UPCAST (PCIBridge, dev, d); + br =PCI_BRIDGE (d); - qdev = &br->dev.qdev; + qdev =DEVICE (d);pci_bridge_map_irq (br, bus_name, map_irq);qdev_prop_set_uint8 (qdev,"port" , port);qdev_init_nofail (qdev);
@@ -134,16 +134,17 @@ static const VMStateDescription vmstate_xio3130_upstream = { .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField []) { -VMSTATE_PCIE_DEVICE (br.dev, PCIEPort), -VMSTATE_STRUCT (br.dev.exp.aer_log, PCIEPort,0 , vmstate_pcie_aer_log, - PCIEAERLog), +VMSTATE_PCIE_DEVICE (br.parent_obj, PCIEPort), +VMSTATE_STRUCT (br.parent_obj.exp.aer_log, PCIEPort,0 , + vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };static Property xio3130_upstream_properties[] = {DEFINE_PROP_UINT8 ("port" , PCIEPort, port,0 ), -DEFINE_PROP_UINT16 ("aer_log_max" , PCIEPort, br.dev.exp.aer_log.log_max, +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 (), };
@@ -169,7 +170,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_upstream_info = { .name ="x3130-upstream" , - .parent =TYPE_PCI_DEVICE , + .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (PCIEPort), .class_init = xio3130_upstream_class_init, };
hw/pci-host/apb.c +2 -2
@@ -464,7 +464,7 @@ PCIBus *pci_apb_init(hwaddr special_base,/* APB secondary busses */ pci_dev =pci_create_multifunction (d->bus,PCI_DEVFN (1 ,0 ), true,"pbm-bridge" ); - br =DO_UPCAST (PCIBridge, dev, pci_dev); + br =PCI_BRIDGE (pci_dev);pci_bridge_map_irq (br,"Advanced PCI Bus secondary bridge 1" , pci_apb_map_irq);qdev_init_nofail (&pci_dev->qdev);
@@ -472,7 +472,7 @@ PCIBus *pci_apb_init(hwaddr special_base, pci_dev =pci_create_multifunction (d->bus,PCI_DEVFN (1 ,1 ), true,"pbm-bridge" ); - br =DO_UPCAST (PCIBridge, dev, pci_dev); + br =PCI_BRIDGE (pci_dev);pci_bridge_map_irq (br,"Advanced PCI Bus secondary bridge 2" , pci_apb_map_irq);qdev_init_nofail (&pci_dev->qdev);
hw/pci/pci_bridge.c +29 -11
@@ -141,8 +141,9 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias, MemoryRegion *parent_space, bool enabled) { -pcibus_t base =pci_bridge_get_base (&bridge->dev, type); -pcibus_t limit =pci_bridge_get_limit (&bridge->dev, type); +PCIDevice *bridge_dev =PCI_DEVICE (bridge); +pcibus_t base =pci_bridge_get_base (bridge_dev, type); +pcibus_t limit =pci_bridge_get_limit (bridge_dev, type);/* TODO: this doesn't handle base = 0 limit = 2^64 - 1 correctly. * Apparently no way to do this with existing memory APIs. */ pcibus_t size = enabled && limit >= base ? limit +1 - base :0 ;
@@ -154,7 +155,8 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,static void pci_bridge_init_vga_aliases (PCIBridge *br,PCIBus *parent,MemoryRegion *alias_vga) { -uint16_t brctl =pci_get_word (br->dev.config +PCI_BRIDGE_CONTROL ); +PCIDevice *pd =PCI_DEVICE (br); +uint16_t brctl =pci_get_word (pd->config +PCI_BRIDGE_CONTROL ); memory_region_init_alias(&alias_vga[QEMU_PCI_VGA_IO_LO], OBJECT(br), "pci_bridge_vga_io_lo", &br->address_space_io,
@@ -167,7 +169,7 @@ static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent, QEMU_PCI_VGA_MEM_BASE, QEMU_PCI_VGA_MEM_SIZE);if (brctl &PCI_BRIDGE_CTL_VGA ) { -pci_register_vga (&br->dev, &alias_vga[QEMU_PCI_VGA_MEM ], +pci_register_vga (pd, &alias_vga[QEMU_PCI_VGA_MEM ], &alias_vga[QEMU_PCI_VGA_IO_LO ], &alias_vga[QEMU_PCI_VGA_IO_HI ]); }
@@ -175,9 +177,10 @@ static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent,static PCIBridgeWindows *pci_bridge_region_init (PCIBridge *br) { -PCIBus *parent = br->dev.bus; +PCIDevice *pd =PCI_DEVICE (br); +PCIBus *parent = pd->bus;PCIBridgeWindows *w =g_new (PCIBridgeWindows,1 ); -uint16_t cmd =pci_get_word (br->dev.config +PCI_COMMAND ); +uint16_t cmd =pci_get_word (pd->config +PCI_COMMAND ); pci_bridge_init_alias(br, &w->alias_pref_mem, PCI_BASE_ADDRESS_MEM_PREFETCH,
@@ -205,12 +208,13 @@ static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)static void pci_bridge_region_del (PCIBridge *br,PCIBridgeWindows *w) { -PCIBus *parent = br->dev.bus; +PCIDevice *pd =PCI_DEVICE (br); +PCIBus *parent = pd->bus;memory_region_del_subregion (parent->address_space_io, &w->alias_io);memory_region_del_subregion (parent->address_space_mem, &w->alias_mem);memory_region_del_subregion (parent->address_space_mem, &w->alias_pref_mem); -pci_unregister_vga (&br->dev); +pci_unregister_vga (pd); }static void pci_bridge_region_cleanup (PCIBridge *br,PCIBridgeWindows *w)
@@ -241,7 +245,7 @@ void pci_bridge_update_mappings(PCIBridge *br)void pci_bridge_write_config (PCIDevice *d,uint32_t address,uint32_t val,int len) { -PCIBridge *s =container_of (d, PCIBridge, dev); +PCIBridge *s =PCI_BRIDGE (d);uint16_t oldctl =pci_get_word (d->config +PCI_BRIDGE_CONTROL );uint16_t newctl;
@@ -331,7 +335,7 @@ void pci_bridge_reset(DeviceState *qdev)int pci_bridge_initfn (PCIDevice *dev,const char *typename) {PCIBus *parent = dev->bus; -PCIBridge *br =DO_UPCAST (PCIBridge, dev, dev); +PCIBridge *br =PCI_BRIDGE (dev);PCIBus *sec_bus = &br->sec_bus; pci_word_test_and_set_mask(dev->config + PCI_STATUS,
@@ -379,7 +383,7 @@ int pci_bridge_initfn(PCIDevice *dev, const char *typename)/* default qdev clean up function for PCI-to-PCI bridge */ void pci_bridge_exitfn (PCIDevice *pci_dev) { -PCIBridge *s =DO_UPCAST (PCIBridge, dev, pci_dev); +PCIBridge *s =PCI_BRIDGE (pci_dev);assert (QLIST_EMPTY (&s->sec_bus.child));QLIST_REMOVE (&s->sec_bus, sibling);pci_bridge_region_del (s, s->windows);
@@ -400,3 +404,17 @@ void pci_bridge_map_irq(PCIBridge *br, const char* bus_name, br->map_irq = map_irq; br->bus_name = bus_name; } + +static const TypeInfo pci_bridge_type_info = { + .name =TYPE_PCI_BRIDGE , + .parent =TYPE_PCI_DEVICE , + .instance_size =sizeof (PCIBridge), + .abstract = true, +}; + +static void pci_bridge_register_types (void ) +{ +type_register_static (&pci_bridge_type_info); +} + +type_init (pci_bridge_register_types)
hw/pci/pcie.c +1 -1
@@ -305,7 +305,7 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot) dev->exp.hpev_notified = false; -pci_bus_hotplug (pci_bridge_get_sec_bus (DO_UPCAST (PCIBridge, dev, dev)), +pci_bus_hotplug (pci_bridge_get_sec_bus (PCI_BRIDGE (dev)), pcie_cap_slot_hotplug, &dev->qdev); }
include/hw/pci/pci_bus.h +6 -1
@@ -53,8 +53,13 @@ struct PCIBridgeWindows {MemoryRegion alias_vga[QEMU_PCI_VGA_NUM_REGIONS ]; }; +#define TYPE_PCI_BRIDGE "base-pci-bridge" +#define PCI_BRIDGE (obj) OBJECT_CHECK(PCIBridge, (obj), TYPE_PCI_BRIDGE) +struct PCIBridge { -PCIDevice dev; +/*< private >*/ +PCIDevice parent_obj; +/*< public >*/ /* private member */ PCIBus sec_bus;
Signature · 1 line
1.8.1.4
RFC PATCH 2/4 pci-bridge-dev: QOM parent field cleanup
In reply to Andreas Färber--- hw/pci-bridge/pci_bridge_dev.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
hw/pci-bridge/pci_bridge_dev.c +14 -8
@@ -27,8 +27,15 @@#include "exec/memory.h" #include "hw/pci/pci_bus.h" +#define TYPE_PCI_BRIDGE_DEV "pci-bridge" +#define PCI_BRIDGE_DEV (obj) \ + OBJECT_CHECK(PCIBridgeDev, (obj), TYPE_PCI_BRIDGE_DEV) +struct PCIBridgeDev { -PCIBridge bridge; +/*< private >*/ +PCIBridge parent_obj; +/*< public >*/ +MemoryRegion bar;uint8_t chassis_nr;#define PCI_BRIDGE_DEV_F_MSI_REQ 0
@@ -39,7 +46,7 @@ typedef struct PCIBridgeDev PCIBridgeDev;static int pci_bridge_dev_initfn (PCIDevice *dev) {PCIBridge *br =PCI_BRIDGE (dev); -PCIBridgeDev *bridge_dev =DO_UPCAST (PCIBridgeDev, bridge, br); +PCIBridgeDev *bridge_dev =PCI_BRIDGE_DEV (dev);int err; err =pci_bridge_initfn (dev,TYPE_PCI_BUS );
@@ -81,8 +88,7 @@ bridge_error:static void pci_bridge_dev_exitfn (PCIDevice *dev) { -PCIBridge *br =PCI_BRIDGE (dev); -PCIBridgeDev *bridge_dev =DO_UPCAST (PCIBridgeDev, bridge, br); +PCIBridgeDev *bridge_dev =PCI_BRIDGE_DEV (dev);if (msi_present (dev)) {msi_uninit (dev); }
@@ -104,7 +110,7 @@ static void pci_bridge_dev_write_config(PCIDevice *d,static void qdev_pci_bridge_dev_reset (DeviceState *qdev) { -PCIDevice *dev =DO_UPCAST (PCIDevice, qdev, qdev); +PCIDevice *dev =PCI_DEVICE (qdev);pci_bridge_reset (qdev);shpc_reset (dev);
@@ -120,8 +126,8 @@ static Property pci_bridge_dev_properties[] = {static const VMStateDescription pci_bridge_dev_vmstate = { .name ="pci_bridge" , .fields = (VMStateField []) { -VMSTATE_PCI_DEVICE (bridge.parent_obj, PCIBridgeDev), -SHPC_VMSTATE (bridge.parent_obj.shpc, PCIBridgeDev), +VMSTATE_PCI_DEVICE (parent_obj, PCIBridge), +SHPC_VMSTATE (shpc, PCIDevice),VMSTATE_END_OF_LIST () } };
@@ -144,7 +150,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data) }static const TypeInfo pci_bridge_dev_info = { - .name ="pci-bridge" , + .name =TYPE_PCI_BRIDGE_DEV , .parent =TYPE_PCI_BRIDGE , .instance_size =sizeof (PCIBridgeDev), .class_init = pci_bridge_dev_class_init,
Signature · 1 line
1.8.1.4
RFC PATCH 3/4 pci-bridge/i82801b11: Rename parent field
In reply to Andreas Färber--- hw/pci-bridge/i82801b11.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
hw/pci-bridge/i82801b11.c +3 -1
@@ -52,7 +52,9 @@#define I82801ba_SSVID_SSID 0typedef struct I82801b11Bridge { -PCIBridge br; +/*< private >*/ +PCIBridge parent_obj; +/*< public >*/ }I82801b11Bridge ;static int i82801b11_bridge_initfn (PCIDevice *d)
Signature · 1 line
1.8.1.4
RFC PATCH 4/4 pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
In reply to Andreas Färber--- 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(-)
hw/pci-bridge/ioh3420.c +10 -13
@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,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 ("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 (), };
@@ -228,7 +225,7 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)static const TypeInfo ioh3420_info = { .name ="ioh3420" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = ioh3420_class_init, };
hw/pci-bridge/xio3130_downstream.c +10 -13
@@ -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,19 +154,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,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 ("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 (), };
@@ -195,7 +192,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_downstream_info = { .name ="xio3130-downstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = xio3130_downstream_class_init, };
hw/pci-bridge/xio3130_upstream.c +7 -8
@@ -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,8 +133,8 @@ 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, 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_upstream = {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, +DEFINE_PROP_UINT16 ("aer_log_max" , PCIDevice, + exp.aer_log.log_max,PCIE_AER_LOG_MAX_DEFAULT ),DEFINE_PROP_END_OF_LIST (), };
@@ -170,7 +169,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_upstream_info = { .name ="x3130-upstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_PORT , .instance_size =sizeof (PCIEPort), .class_init = xio3130_upstream_class_init, };
hw/pci/pcie_port.c +22 -0
@@ -116,3 +116,25 @@ void pcie_chassis_del_slot(PCIESlot *s) {QLIST_REMOVE (s, next); } + +static const TypeInfo pcie_port_type_info = { + .name =TYPE_PCIE_PORT , + .parent =TYPE_PCI_BRIDGE , + .instance_size =sizeof (PCIEPort), + .abstract = true, +}; + +static const TypeInfo pcie_slot_type_info = { + .name =TYPE_PCIE_SLOT , + .parent =TYPE_PCIE_PORT , + .instance_size =sizeof (PCIESlot), + .abstract = 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)
include/hw/pci/pcie_port.h +12 -2
@@ -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;
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Quoted 60 lines · Andreas Färber
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-10@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };static Property ioh3420_properties[] = { -DEFINE_PROP_UINT8 ("port" , PCIESlot, port.port,0 ),
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 10 lines
hw/pci-bridge/ioh3420.c +3
-2+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?
Quoted 60 lines
hw/pci-bridge/ioh3420.c +1
-1@@ -228,7 +225,7 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)static const TypeInfo ioh3420_info = { .name ="ioh3420" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = ioh3420_class_init, };hw/pci-bridge/xio3130_downstream.c +6
-10@@ -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,19 +154,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };static Property xio3130_downstream_properties[] = { -DEFINE_PROP_UINT8 ("port" , PCIESlot, port.port,0 ),
hw/pci-bridge/xio3130_downstream.c static const VMStateDescription vmstate_xio3130_downstream = { · line 172
Quoted 10 lines
hw/pci-bridge/xio3130_downstream.c +3
-2+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.
Quoted 44 lines
hw/pci-bridge/xio3130_downstream.c +1
-1@@ -195,7 +192,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_downstream_info = { .name ="xio3130-downstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = xio3130_downstream_class_init, };hw/pci-bridge/xio3130_upstream.c +4
-5@@ -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,8 +133,8 @@ 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, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () }
hw/pci-bridge/xio3130_upstream.c static const VMStateDescription vmstate_xio3130_upstream = { · line 149
Quoted 10 lines
hw/pci-bridge/xio3130_upstream.c +2
-2@@ -143,8 +142,8 @@ static const VMStateDescription vmstate_xio3130_upstream = {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, +DEFINE_PROP_UINT16 ("aer_log_max" , PCIDevice, + exp.aer_log.log_max,PCIE_AER_LOG_MAX_DEFAULT ),DEFINE_PROP_END_OF_LIST (), };
Same question.
Quoted 75 lines
hw/pci-bridge/xio3130_upstream.c +1
-1@@ -170,7 +169,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_upstream_info = { .name ="x3130-upstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_PORT , .instance_size =sizeof (PCIEPort), .class_init = xio3130_upstream_class_init, };hw/pci/pcie_port.c +22
-0@@ -116,3 +116,25 @@ void pcie_chassis_del_slot(PCIESlot *s) {QLIST_REMOVE (s, next); } + +static const TypeInfo pcie_port_type_info = { + .name =TYPE_PCIE_PORT , + .parent =TYPE_PCI_BRIDGE , + .instance_size =sizeof (PCIEPort), + .abstract = true, +}; + +static const TypeInfo pcie_slot_type_info = { + .name =TYPE_PCIE_SLOT , + .parent =TYPE_PCIE_PORT , + .instance_size =sizeof (PCIESlot), + .abstract = 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)include/hw/pci/pcie_port.h +12
-2@@ -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;Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Am 21.07.2013 22:26, schrieb Michael S. Tsirkin:
Quoted 59 lines · Andreas Färber
Quoted 58 lines
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-9@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };
Quoted 16 lines
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 12 lines
hw/pci-bridge/ioh3420.c +3
-3static Property ioh3420_properties[] = { -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.
It doesn't. AFAIU both VMStateDescription and qdev properties store a numerical offset from the object pointer. CC'ing Juan and Paolo/Anthony.
I.e., if I don't find a counter-example we can drop the arguments from VMSTATE_PCI_DEVICE() and VMSTATE_PCIE_DEVICE() completely since the offset would always be 0 and the field name can be hardcoded, cf. VMSTATE_CPU() in qom/cpu.h.
Quoted 1 line
What are the advantages of this hack?
The concrete problems I encountered in this series were a) 80 chars limit and b) parent_obj.parent_obj.foo.
Since offsetof(MyState, parent_obj.foo) == offsetof(ParentState, foo), this was a neat way to shorten the line.
The main point of the series is introducing abstract QOM types matching a specific struct. Renaming the parent field helps catch users that should no longer be accessing it, both in compile-testing and in future patch review.
What we would do about VMState when someone at some future point wants to move PCI state into a QOM interface I have no clue, I'll let the person who throws the first stone worry about that. ;)
Regards,
Andreas
Quoted 208 lines
Quoted 60 lines
hw/pci-bridge/ioh3420.c +1
-1@@ -228,7 +225,7 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)static const TypeInfo ioh3420_info = { .name ="ioh3420" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = ioh3420_class_init, };hw/pci-bridge/xio3130_downstream.c +6
-10@@ -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,19 +154,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };static Property xio3130_downstream_properties[] = { -DEFINE_PROP_UINT8 ("port" , PCIESlot, port.port,0 ),hw/pci-bridge/xio3130_downstream.c static const VMStateDescription vmstate_xio3130_downstream = { · line 172
Quoted 10 lines
hw/pci-bridge/xio3130_downstream.c +3
-2+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.
Quoted 44 lines
hw/pci-bridge/xio3130_downstream.c +1
-1@@ -195,7 +192,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_downstream_info = { .name ="xio3130-downstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = xio3130_downstream_class_init, };hw/pci-bridge/xio3130_upstream.c +4
-5@@ -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,8 +133,8 @@ 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, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () }hw/pci-bridge/xio3130_upstream.c static const VMStateDescription vmstate_xio3130_upstream = { · line 149
Quoted 10 lines
hw/pci-bridge/xio3130_upstream.c +2
-2@@ -143,8 +142,8 @@ static const VMStateDescription vmstate_xio3130_upstream = {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, +DEFINE_PROP_UINT16 ("aer_log_max" , PCIDevice, + exp.aer_log.log_max,PCIE_AER_LOG_MAX_DEFAULT ),DEFINE_PROP_END_OF_LIST (), };Same question.
Quoted 75 lines
hw/pci-bridge/xio3130_upstream.c +1
-1@@ -170,7 +169,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_upstream_info = { .name ="x3130-upstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_PORT , .instance_size =sizeof (PCIEPort), .class_init = xio3130_upstream_class_init, };hw/pci/pcie_port.c +22
-0@@ -116,3 +116,25 @@ void pcie_chassis_del_slot(PCIESlot *s) {QLIST_REMOVE (s, next); } + +static const TypeInfo pcie_port_type_info = { + .name =TYPE_PCIE_PORT , + .parent =TYPE_PCI_BRIDGE , + .instance_size =sizeof (PCIEPort), + .abstract = true, +}; + +static const TypeInfo pcie_slot_type_info = { + .name =TYPE_PCIE_SLOT , + .parent =TYPE_PCIE_PORT , + .instance_size =sizeof (PCIESlot), + .abstract = 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)include/hw/pci/pcie_port.h +12
-2@@ -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;Signature · 1 line
1.8.1.4
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Quoted 77 lines · Andreas Färber
Am 21.07.2013 22:26, schrieb Michael S. Tsirkin:
Quoted 59 lines · Andreas Färber
Quoted 58 lines
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-9@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };Quoted 16 lines
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 12 lines
hw/pci-bridge/ioh3420.c +3
-3static Property ioh3420_properties[] = { -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.
Quoted 7 lines
It doesn't. AFAIU both VMStateDescription and qdev properties store a numerical offset from the object pointer. CC'ing Juan and Paolo/Anthony.
I.e., if I don't find a counter-example we can drop the arguments from
VMSTATE_PCI_DEVICE()andVMSTATE_PCIE_DEVICE()completely since the offset would always be 0 and the field name can be hardcoded, cf.VMSTATE_CPU()inqom/cpu.h.
Currently VMSTATE macros are safe since they all use the same structure.
You are making them use different types with no compile or runtime checking that the correct type is used.
Quoted 7 lines
Quoted 2 lines
What are the advantages of this hack?
The concrete problems I encountered in this series were a) 80 chars limit and b) parent_obj.parent_obj.foo.
Sinceoffsetof(MyState, parent_obj.foo)==offsetof(ParentState, foo), this was a neat way to shorten the line.
I don't think it's a good reason.
Quoted 11 lines
The main point of the series is introducing abstract QOM types matching a specific struct. Renaming the parent field helps catch users that should no longer be accessing it, both in compile-testing and in future patch review.
What we would do about VMState when someone at some future point wants to move PCI state into a QOM interface I have no clue, I'll let the person who throws the first stone worry about that. ;)
Regards,
Andreas
NACK
Please find a way to have code that won't silently break when we refactor code.
Normal QOM type use has runtime checks to address that.
Quoted 213 lines
Quoted 208 lines
Quoted 60 lines
hw/pci-bridge/ioh3420.c +1
-1@@ -228,7 +225,7 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)static const TypeInfo ioh3420_info = { .name ="ioh3420" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = ioh3420_class_init, };hw/pci-bridge/xio3130_downstream.c +6
-10@@ -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,19 +154,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };static Property xio3130_downstream_properties[] = { -DEFINE_PROP_UINT8 ("port" , PCIESlot, port.port,0 ),hw/pci-bridge/xio3130_downstream.c static const VMStateDescription vmstate_xio3130_downstream = { · line 172
Quoted 10 lines
hw/pci-bridge/xio3130_downstream.c +3
-2+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.
Quoted 44 lines
hw/pci-bridge/xio3130_downstream.c +1
-1@@ -195,7 +192,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_downstream_info = { .name ="xio3130-downstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = xio3130_downstream_class_init, };hw/pci-bridge/xio3130_upstream.c +4
-5@@ -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,8 +133,8 @@ 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, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () }hw/pci-bridge/xio3130_upstream.c static const VMStateDescription vmstate_xio3130_upstream = { · line 149
Quoted 10 lines
hw/pci-bridge/xio3130_upstream.c +2
-2@@ -143,8 +142,8 @@ static const VMStateDescription vmstate_xio3130_upstream = {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, +DEFINE_PROP_UINT16 ("aer_log_max" , PCIDevice, + exp.aer_log.log_max,PCIE_AER_LOG_MAX_DEFAULT ),DEFINE_PROP_END_OF_LIST (), };Same question.
Quoted 75 lines
hw/pci-bridge/xio3130_upstream.c +1
-1@@ -170,7 +169,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_upstream_info = { .name ="x3130-upstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_PORT , .instance_size =sizeof (PCIEPort), .class_init = xio3130_upstream_class_init, };hw/pci/pcie_port.c +22
-0@@ -116,3 +116,25 @@ void pcie_chassis_del_slot(PCIESlot *s) {QLIST_REMOVE (s, next); } + +static const TypeInfo pcie_port_type_info = { + .name =TYPE_PCIE_PORT , + .parent =TYPE_PCI_BRIDGE , + .instance_size =sizeof (PCIEPort), + .abstract = true, +}; + +static const TypeInfo pcie_slot_type_info = { + .name =TYPE_PCIE_SLOT , + .parent =TYPE_PCIE_PORT , + .instance_size =sizeof (PCIESlot), + .abstract = 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)include/hw/pci/pcie_port.h +12
-2@@ -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;Signature · 1 line
1.8.1.4Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
In reply to Michael S. TsirkinQuoted 59 lines · Michael S. Tsirkin
Quoted 58 lines · Andreas Färber
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-9@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };
Quoted 16 lines
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 12 lines
hw/pci-bridge/ioh3420.c +3
-3static Property ioh3420_properties[] = { -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 cast?
VMstate takes a void *.
One an object is cast to a void *, it's just as much as PCIESlot as it is a PCIEPort.
That said, for consistency, I think having everything be relatively to *one* type for a Property list is pretty helpful.
Expecting someone to know the type hierarchy by heart such that this doesn't look like a bug is too much IMHO.
Regards,
Anthony Liguori
Quoted 211 lines
What are the advantages of this hack?
Quoted 60 lines
hw/pci-bridge/ioh3420.c +1
-1@@ -228,7 +225,7 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)static const TypeInfo ioh3420_info = { .name ="ioh3420" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = ioh3420_class_init, };hw/pci-bridge/xio3130_downstream.c +6
-10@@ -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,19 +154,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };static Property xio3130_downstream_properties[] = { -DEFINE_PROP_UINT8 ("port" , PCIESlot, port.port,0 ),hw/pci-bridge/xio3130_downstream.c static const VMStateDescription vmstate_xio3130_downstream = { · line 172
Quoted 10 lines
hw/pci-bridge/xio3130_downstream.c +3
-2+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.
Quoted 44 lines
hw/pci-bridge/xio3130_downstream.c +1
-1@@ -195,7 +192,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_downstream_info = { .name ="xio3130-downstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_SLOT , .instance_size =sizeof (PCIESlot), .class_init = xio3130_downstream_class_init, };hw/pci-bridge/xio3130_upstream.c +4
-5@@ -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,8 +133,8 @@ 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, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () }hw/pci-bridge/xio3130_upstream.c static const VMStateDescription vmstate_xio3130_upstream = { · line 149
Quoted 10 lines
hw/pci-bridge/xio3130_upstream.c +2
-2@@ -143,8 +142,8 @@ static const VMStateDescription vmstate_xio3130_upstream = {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, +DEFINE_PROP_UINT16 ("aer_log_max" , PCIDevice, + exp.aer_log.log_max,PCIE_AER_LOG_MAX_DEFAULT ),DEFINE_PROP_END_OF_LIST (), };Same question.
Quoted 75 lines
hw/pci-bridge/xio3130_upstream.c +1
-1@@ -170,7 +169,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)static const TypeInfo xio3130_upstream_info = { .name ="x3130-upstream" , - .parent =TYPE_PCI_BRIDGE , + .parent =TYPE_PCIE_PORT , .instance_size =sizeof (PCIEPort), .class_init = xio3130_upstream_class_init, };hw/pci/pcie_port.c +22
-0@@ -116,3 +116,25 @@ void pcie_chassis_del_slot(PCIESlot *s) {QLIST_REMOVE (s, next); } + +static const TypeInfo pcie_port_type_info = { + .name =TYPE_PCIE_PORT , + .parent =TYPE_PCI_BRIDGE , + .instance_size =sizeof (PCIEPort), + .abstract = true, +}; + +static const TypeInfo pcie_slot_type_info = { + .name =TYPE_PCIE_SLOT , + .parent =TYPE_PCIE_PORT , + .instance_size =sizeof (PCIESlot), + .abstract = 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)include/hw/pci/pcie_port.h +12
-2@@ -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;Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Am 22.07.2013 22:29, schrieb Anthony Liguori:
Quoted 82 lines
Quoted 59 lines · Michael S. Tsirkin
Quoted 58 lines · Andreas Färber
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-9@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };Quoted 16 lines
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 12 lines
hw/pci-bridge/ioh3420.c +3
-3static Property ioh3420_properties[] = { -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 cast?
VMstate takes a void *.
Quoted 8 lines
One an object is cast to a void *, it's just as much as PCIESlot as it is a PCIEPort.
That said, for consistency, I think having everything be relatively to *one* type for a Property list is pretty helpful.
Expecting someone to know the type hierarchy by heart such that this doesn't look like a bug is too much IMHO.
I'm updating the patch to that effect for VMState. But I notice the real fix for qdev properties would be to move the PCIEPort property to the new PCIEPort type, so that all derived types inherit it. :)
For VMState I believe the real follow-up fix would be mst defining a central macro VMSTATE_PCI_DEVICE_AER_LOG() operating on PCIDevice. Why is that separate from VMSTATE_PCI_DEVICE() or VMSTATE_PCIE_DEVICE() in the first place?
Regards,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Quoted 96 lines · Andreas Färber
Am 22.07.2013 22:29, schrieb Anthony Liguori:
Quoted 82 lines
Quoted 59 lines · Michael S. Tsirkin
Quoted 58 lines · Andreas Färber
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-9@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };Quoted 16 lines
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 12 lines
hw/pci-bridge/ioh3420.c +3
-3static Property ioh3420_properties[] = { -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 cast?
VMstate takes a void *.
Quoted 8 lines
One an object is cast to a void *, it's just as much as PCIESlot as it is a PCIEPort.
That said, for consistency, I think having everything be relatively to *one* type for a Property list is pretty helpful.
Expecting someone to know the type hierarchy by heart such that this doesn't look like a bug is too much IMHO.
I'm updating the patch to that effect for VMState. But I notice the real fix for qdev properties would be to move the PCIEPort property to the new PCIEPort type, so that all derived types inherit it. :)
Quoted 7 lines
For VMState I believe the real follow-up fix would be mst defining a central macro
VMSTATE_PCI_DEVICE_AER_LOG()operating on PCIDevice. Why is that separate fromVMSTATE_PCI_DEVICE()orVMSTATE_PCIE_DEVICE()in the first place?Regards,
Andreas
The real fix is savevm/loadvm taking into account
the class hierarchy.
Quoted 3 lines
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Am 23.07.2013 09:07, schrieb Michael S. Tsirkin:
Quoted 8 lines · Andreas Färber
Quoted 4 lines
For VMState I believe the real follow-up fix would be mst defining a central macro
VMSTATE_PCI_DEVICE_AER_LOG()operating on PCIDevice. Why is that separate fromVMSTATE_PCI_DEVICE()orVMSTATE_PCIE_DEVICE()in the first place?The real fix is savevm/loadvm taking into account
the class hierarchy.
That's not helping, unless you write a patch to show what you mean and how that is going to be migration-compatible.
Does your not answering the question mean you don't know?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Quoted 11 lines · Andreas Färber
Am 23.07.2013 09:07, schrieb Michael S. Tsirkin:
Quoted 8 lines · Andreas Färber
Quoted 4 lines
For VMState I believe the real follow-up fix would be mst defining a central macro
VMSTATE_PCI_DEVICE_AER_LOG()operating on PCIDevice. Why is that separate fromVMSTATE_PCI_DEVICE()orVMSTATE_PCIE_DEVICE()in the first place?The real fix is savevm/loadvm taking into account
the class hierarchy.That's not helping, unless you write a patch to show what you mean and
I merely mean that if I inherit a class I should inherit it's vmstate.
So explicitly adding VMSTATE_PCI_DEVICE should not be necessary.
Quoted 1 line
how that is going to be migration-compatible.
Most devices put VMSTATE_PCI_DEVICE at the beninning, so just calling that before vmstate for the device should be compatible.
Quoted 3 lines
Does your not answering the question mean you don't know?
Andreas
I think the answer is that most pcie devices don't implement AER. AFAIK PCI devices can't support AER at all.
Quoted 3 lines
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Am 23.07.2013 11:59, schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Am 23.07.2013 09:07, schrieb Michael S. Tsirkin:
Quoted 5 lines · Andreas Färber
Quoted 4 lines
For VMState I believe the real follow-up fix would be mst defining a central macro
VMSTATE_PCI_DEVICE_AER_LOG()operating on PCIDevice. Why is that separate fromVMSTATE_PCI_DEVICE()orVMSTATE_PCIE_DEVICE()in the first place?I think the answer is that most pcie devices
don't implement AER. AFAIK PCI devices can't
support AER at all.
Okay, so if it's just PCIe, then XHCI is the oddball preventing moving it into VMSTATE_PCIE_DEVICE(). XHCI has VMSTATE_MSIX() in its place, also operating on PCIDevice.
Is there a way to detect use of AER or MSIX to place those into subsections of VMSTATE_PCIE_DEVICE()?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Hi,
Quoted 3 lines
Okay, so if it's just PCIe, then XHCI is the oddball preventing moving it into
VMSTATE_PCIE_DEVICE(). XHCI hasVMSTATE_MSIX()in its place, also operating on PCIDevice.
Given that live migration support for xhci was added post-1.5 so we don't have a release with it yet this shouldn't be a blocker in case we get this sorted in time for 1.6.
Quoted 2 lines
Is there a way to detect use of AER or MSIX to place those into subsections of
VMSTATE_PCIE_DEVICE()?
There is msix_enabled() ...
Dunno about AER.
cheers,
Gerd
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Quoted 6 lines · Gerd Hoffmann
Hi,
Quoted 3 lines
Okay, so if it's just PCIe, then XHCI is the oddball preventing moving it into
VMSTATE_PCIE_DEVICE(). XHCI hasVMSTATE_MSIX()in its place, also operating on PCIDevice.
Quoted 8 lines
Given that live migration support for xhci was added post-1.5 so we don't have a release with it yet this shouldn't be a blocker in case we get this sorted in time for 1.6.
Quoted 2 lines
Is there a way to detect use of AER or MSIX to place those into subsections of
VMSTATE_PCIE_DEVICE()?There is
msix_enabled()...
msix_present()
Quoted 4 lines
Dunno about AER.
cheers,
Gerd
Not at the moment but it's not hard to add.
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
In reply to Anthony LiguoriQuoted 88 lines · Anthony Liguori
Quoted 59 lines · Michael S. Tsirkin
Quoted 58 lines · Andreas Färber
Signed-off-by Andreas Färber <[email protected]>--- 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(-)hw/pci-bridge/ioh3420.c +6
-9@@ -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,19 +187,19 @@ 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_DEVICE (parent_obj, PCIBridge), +VMSTATE_STRUCT (exp.aer_log, PCIDevice,0 , vmstate_pcie_aer_log, PCIEAERLog),VMSTATE_END_OF_LIST () } };Quoted 16 lines
hw/pci-bridge/ioh3420.c static const VMStateDescription vmstate_ioh3420 = { · line 205
Quoted 12 lines
hw/pci-bridge/ioh3420.c +3
-3static Property ioh3420_properties[] = { -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 cast?
VMstate takes a void *.
One an object is cast to a void *, it's just as much as PCIESlot as it is a PCIEPort.
That said, for consistency, I think having everything be relatively to *one* type for a Property list is pretty helpful.
Quoted 6 lines
Expecting someone to know the type hierarchy by heart such that this doesn't look like a bug is too much IMHO.
Regards,
Anthony Liguori
Exactly.
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
In reply to Anthony LiguoriHi Anthony,
Am 22.07.2013 22:29, schrieb Anthony Liguori:
Quoted 5 lines
for consistency, I think having everything be relatively to *one* type for a Property list is pretty helpful.
Expecting someone to know the type hierarchy by heart such that this doesn't look like a bug is too much IMHO.
I have changed v2 not to mix different-but-compatible struct types in one VMStateDescription.
Could you clarify if that was what you meant with the above?
Or would you also be opposed to - post-1.6 - changing VMSTATE_PCIE_DEVICE(parent_obj[.parent_obj], MyStruct) toVMSTATE_PCIE_DEVICE()
as suggested elsewhere in this thread?
I'm thinking that writing VMSTATE_PCIE_DEVICE() already clearly indicates the developer knows the device inherits from TYPE_PCI_DEVICE.
All PCIe devices using VMSTATE_PCIE_DEVICE() today use it at an offset of 0 and so do all PCI devices using VMSTATE_PCI_DEVICE() apparently. VMSTATE_PCI_DEVICE_POINTER() would be unaffected, but is unused anyway.
My survey also concluded that luckily all VMSTATE_PCIE_DEVICE() and VMSTATE_PCI_DEVICE() are placed as first VMStateField, so moving parent state to its class might be possible, similar to qdev props todays with class_base_init clearing it for derived types.
However this would require to either refactor core VMState code to operate on a list, aggregated from one or more zero-terminated arrays, which I would consider invasive and error-prone, or simply have Device code allocate a new VMStateDescription before registering it in QOM realize (so it can be free'd on unrealize). Thoughts?
Either way, it would work for CPU but not for PCI, since there are two different macros, VMSTATE_PCI_DEVICE() and VMSTATE_PCIE_DEVICE() both for PCIDeviceClass. Not sure how to solve that without multi-inheritence.
SHPC_VMSTATE() seems to be another macro beyond VMSTATE_MSIX() operating on PCIDevice but placed in an individual device (pci-bridge-dev). Can it be turned into a subsection, Michael?
Regards,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types
Quoted 37 lines · Andreas Färber
Hi Anthony,
Am 22.07.2013 22:29, schrieb Anthony Liguori:
Quoted 5 lines
for consistency, I think having everything be relatively to *one* type for a Property list is pretty helpful.
Expecting someone to know the type hierarchy by heart such that this doesn't look like a bug is too much IMHO.
I have changed v2 not to mix different-but-compatible struct types in one VMStateDescription.
Could you clarify if that was what you meant with the above?
Or would you also be opposed to - post-1.6 - changing
VMSTATE_PCIE_DEVICE(parent_obj[.parent_obj], MyStruct)toVMSTATE_PCIE_DEVICE()
as suggested elsewhere in this thread?I'm thinking that writing
VMSTATE_PCIE_DEVICE()already clearly indicates the developer knows the device inherits fromTYPE_PCI_DEVICE.All PCIe devices using
VMSTATE_PCIE_DEVICE()today use it at an offset of 0 and so do all PCI devices usingVMSTATE_PCI_DEVICE()apparently.VMSTATE_PCI_DEVICE_POINTER()would be unaffected, but is unused anyway.My survey also concluded that luckily all
VMSTATE_PCIE_DEVICE()andVMSTATE_PCI_DEVICE()are placed as first VMStateField, so moving parent state to its class might be possible, similar to qdev props todays withclass_base_initclearing it for derived types.
However this would require to either refactor core VMState code to operate on a list, aggregated from one or more zero-terminated arrays, which I would consider invasive and error-prone, or simply have Device code allocate a new VMStateDescription before registering it in QOM realize (so it can be free'd on unrealize). Thoughts?
Quoted 3 lines
Either way, it would work for CPU but not for PCI, since there are two different macros,
VMSTATE_PCI_DEVICE()andVMSTATE_PCIE_DEVICE()both for PCIDeviceClass. Not sure how to solve that without multi-inheritence.
Maybe combine them and use is_express to select the correct format.
Quoted 6 lines
SHPC_VMSTATE()seems to be another macro beyondVMSTATE_MSIX()operating on PCIDevice but placed in an individual device (pci-bridge-dev). Can it be turned into a subsection, Michael?Regards,
Andreas
Not without breaking cross-version migration I think?
Quoted 3 lines
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 6 lines · Andreas Färber
Hello Michael et al.,
This series turns PCIBridge, PCIEPort and PCIESlot into abstract QOM types, so that we can use QOM casts to obtain a pointer.
Possibly this was prompted by q35's PCIe?
What was prompted?
What's the question exactly?
Quoted 33 lines
Don't remember ATM...
Regards,
AndreasCc Anthony Liguori <[email protected]>Cc Paolo Bonzini <[email protected]>Cc Michael S. Tsirkin <[email protected]>Cc Peter Crosthwaite <[email protected]>Cc Hu Tao <[email protected]>Andreas Färber (4):
pci-bridge: Turn into abstract QOM type
pci-bridge-dev: QOM parent field cleanup pci-bridge/i82801b11: Rename parent field
pcie_port: Turn PCIEPort and PCIESlot into abstract QOM typeshw/pci-bridge/dec.c | 4 ++-- hw/pci-bridge/i82801b11.c | 10 ++++++---- hw/pci-bridge/ioh3420.c | 29 +++++++++++++-------------- hw/pci-bridge/pci_bridge_dev.c | 26 +++++++++++++++---------- hw/pci-bridge/xio3130_downstream.c | 29 +++++++++++++-------------- hw/pci-bridge/xio3130_upstream.c | 20 +++++++++---------- hw/pci-host/apb.c | 4 ++-- hw/pci/pci_bridge.c | 40 +++++++++++++++++++++++++++----------- hw/pci/pcie.c | 2 +- hw/pci/pcie_port.c | 22 +++++++++++++++++++++ include/hw/pci/pci_bus.h | 7 ++++++- include/hw/pci/pcie_port.h | 14 +++++++++++-- 12 files changed, 132 insertions(+), 75 deletions(-)Signature · 1 line
1.8.1.4
Am 21.07.2013 22:26, schrieb Michael S. Tsirkin:
Quoted 9 lines · Andreas Färber
Quoted 6 lines
Hello Michael et al.,
This series turns PCIBridge, PCIEPort and PCIESlot into abstract QOM types, so that we can use QOM casts to obtain a pointer.
Possibly this was prompted by q35's PCIe?
What was prompted?
This refactoring series.
Quoted 1 line
What's the question exactly?
Why I prepared this series on an offline train ride on Wednesday. ;) Not for you to answer - something PCIe must've gotten in the way of some QOM realize conversion but the branch is missing the final patch showing what these types and casts are good for. Since we're in Soft Freeze I rather wanted to flush my queues for review though rather than spend more time puzzling why I did this. :)
Andreas
Quoted 35 lines
Quoted 34 lines
Don't remember ATM...
Regards,
AndreasCc Anthony Liguori <[email protected]>Cc Paolo Bonzini <[email protected]>Cc Michael S. Tsirkin <[email protected]>Cc Peter Crosthwaite <[email protected]>Cc Hu Tao <[email protected]>Andreas Färber (4):
pci-bridge: Turn into abstract QOM type
pci-bridge-dev: QOM parent field cleanup pci-bridge/i82801b11: Rename parent field
pcie_port: Turn PCIEPort and PCIESlot into abstract QOM typeshw/pci-bridge/dec.c | 4 ++-- hw/pci-bridge/i82801b11.c | 10 ++++++---- hw/pci-bridge/ioh3420.c | 29 +++++++++++++-------------- hw/pci-bridge/pci_bridge_dev.c | 26 +++++++++++++++---------- hw/pci-bridge/xio3130_downstream.c | 29 +++++++++++++-------------- hw/pci-bridge/xio3130_upstream.c | 20 +++++++++---------- hw/pci-host/apb.c | 4 ++-- hw/pci/pci_bridge.c | 40 +++++++++++++++++++++++++++----------- hw/pci/pcie.c | 2 +- hw/pci/pcie_port.c | 22 +++++++++++++++++++++ include/hw/pci/pci_bus.h | 7 ++++++- include/hw/pci/pcie_port.h | 14 +++++++++++-- 12 files changed, 132 insertions(+), 75 deletions(-)Signature · 1 line
1.8.1.4
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Am 22.07.2013 19:22, schrieb Andreas Färber:
Quoted 11 lines
Am 21.07.2013 22:26, schrieb Michael S. Tsirkin:
Quoted 9 lines · Andreas Färber
Quoted 6 lines
Hello Michael et al.,
This series turns PCIBridge, PCIEPort and PCIESlot into abstract QOM types, so that we can use QOM casts to obtain a pointer.
Possibly this was prompted by q35's PCIe?
What was prompted?
Quoted 8 lines
This refactoring series.
Quoted 1 line
What's the question exactly?
Why I prepared this series on an offline train ride on Wednesday. ;) Not for you to answer - something PCIe must've gotten in the way of some QOM realize conversion but the branch is missing the final patch showing what these types and casts are good for.
Doh, stumbled over it again: pci_bridge_initfn() should become PCIBridge's realizefn, and walking down the type hierarchy pcie_port_init_reg() should become PCIEPort's (which needs TYPE_PCIE_PORT), etc. That's a bigger refactoring though, so v2 will come without it.
Andreas
Quoted 11 lines
Since we're in Soft Freeze I
rather wanted to flush my queues for review though rather than spend more time puzzling why I did this. :)Andreas
Quoted 5 lines
Quoted 5 lines
Don't remember ATM...
Regards,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg