Hi Andreas, On Sun, Jun 30, 2013 at 8:44 PM, Andreas Färber <afaer...@suse.de> wrote: > Hi Peter, > > Am 24.06.2013 08:49, schrieb peter.crosthwa...@xilinx.com: >> From: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >> >> There are a number of different cast implementations from various >> stages of QEMU development out in device model land. This series cleans >> up the ones involving TYPE_PCI_DEVICE to consistently use proper QOM >> casts for both up and down casts. >> >> Some were easy, some needed QOM cast macros which are added as >> appropriate. >> >> Following the recent discussion RE performance consequences of QOM >> casts, im interested in any reports of possible performance regressions >> here, although I am hoping that Anthony current efforts to improve >> QOM casting efficiency make this a non-issue. > > While I did not run extensive benchmarks, state of the discussion > between Paolo, Anthony and me, I believe, was that it can be considered > okay to use QOM casts "everywhere" consistently now, but we should not > use casts where they are unnecessary (i.e., only where we change type). > E.g., http://patchwork.ozlabs.org/patch/255367/ > > I have therefore dropped some opaque casts where the type on both sides > of void* matched (for an up-/downcast I do prefer the cast for safety). > > Anyway, if we get this merged early, then there is still time for more > benchmarking/optimizations during Soft Freeze IMO. > Maybe our staging tree will facilitate testing, too. ;) > >> Changed since V1: >> Removed hunks which macroified VMSD names >> Dropped virtio/virtio.pci patch >> Rebased >> >> >> Peter Crosthwaite (30): >> net/e1000: QOM Upcast Sweep >> net/rtl8139: QOM Upcast Sweep >> net/pcnet-pci: QOM Upcast Sweep >> usb/hcd-xhci: QOM Upcast Sweep >> scsi/lsi53c895a: QOM Upcast Sweep >> scsi/megasas: QOM Upcast Sweep >> scsi/esp-pci: QOM Upcast Sweep >> ide/ich: QOM Upcast Sweep >> ide/piix: QOM casting sweep >> acpi/piix4: QOM Upcast Sweep >> misc/pci-testdev: QOM Upcast Sweep >> virtio/vmware_vga: QOM casting sweep >> misc/ivshmem: QOM Upcast Sweep >> xen/xen_platform: QOM casting sweep > > As requested, I've started picking up QOM type/cast/realize patches on: > > git://github.com/afaerber/qemu-cpu.git qom-next
Perhaps this is becoming the "qom-devices" queue? > https://github.com/afaerber/qemu-cpu/commits/qom-next > > (Not to be confused with my qom-cpu / qom-cpu-next CPU trees.) > > If anyone wishes to contribute patches against that tree, please > indicate so with --subject-prefix="PATCH qom-next ...". > > As a matter of personal taste and consistency, I've used the gtk-doc > notation DO_UPCAST() wherever I stumbled over it in commit messages. > > I've queued all patches above except for ide/piix (09/30) and had > comments and/or minor changes for some of them. Noticing some > incompleteness, I will reiterate over them. > I think the incompleteness comes from the fact that my series is only worrying about type PCI_DEVICE. I have left alone QOM cast issues for casts that are not to/from or through TYPE_PCI_DEVICE, so there are instances where a QOM cast macro may be introduces but not used. > Whether I send a pull when we're all happy with it or whether we let > submaintainers pick/pull by subsystem at some point doesn't matter to > me, as long as we can join efforts to make QOM realize reality soon. :) > I think getting it all into one giant queue then we can centralise testing a little better. >> isa/*: QOM casting sweep >> pci/*: QOM casting sweep >> pci-bridge/pci_bridge_dev: Don't use DO_UPCAST >> pci-bridge/*: substitute ->qdev casts with DEVICE() >> pci/pci_bridge: substitute ->qdev casts with DEVICE() >> misc/vfio: substitute ->qdev casts with DEVICE() >> net/eepro100: substitute ->qdev casts with DEVICE() >> net/ne2000: substitute ->qdev casts with DEVICE() >> usb/*: substitute ->qdev casts with DEVICE() >> watchdog/wdt_i6300esb: substitute ->qdev casts with DEVICE() >> scsi/vmw_pvscsi: substitute ->qdev casts with DEVICE() >> i2c/smbus_ich9: substitute ->qdev casts with DEVICE() >> ide/cmd646: substitute ->qdev casts with DEVICE() >> ide/via: substitute ->qdev casts with DEVICE() >> pci-host/*: substitute ->qdev casts with DEVICE() >> i386/*: substitute ->qdev casts with DEVICE() > > These patches seem more "sloppy" while not reaching a clear goal such as > dropping a macro or renaming PCIDevice::qdev, so I'd prefer to get open > issues sorted out before rushing ahead with half-done conversions. > Functionally everything I've seen so far looked fine though. > > But maybe I'm missing something? What exactly was the motivation behind > the series? Do you have a follow-up? > I have an experimental series out-of-tree (change the parent type of TYPE_PCI_DEVICE to TYPE_SYSBUS) to remove the need to coreify devices for use as both sysbus and PCI (e.g. EHCI). These casts get in the way of that series. I really want to make it clear though, that this series is legitimate cleanup without that goal, so I didn't want to tangle this series in that much-longer discussion. So this series specifically is a compile bug chase on: diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 6ef1f97..5f607b3 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -217,7 +217,6 @@ typedef void (*MSIVectorPollNotifier)(PCIDevice *dev, unsigned int vector_end); struct PCIDevice { - DeviceState qdev; /* PCI config space */ uint8_t *config; FWIW, one could actually remove all parent_obj fields from QOM structs (just like this) and the tree should compile, good way to catch all legacy casts. Regards, Peter > Regards, > Andreas > >> >> hw/acpi/piix4.c | 31 +++++++++++++++++-------------- >> hw/display/vmware_vga.c | 13 ++++++++----- >> hw/i2c/smbus_ich9.c | 2 +- >> hw/i386/kvm/pci-assign.c | 21 ++++++++++++--------- >> hw/i386/pc.c | 3 ++- >> hw/i386/pc_piix.c | 4 ++-- >> hw/i386/pc_q35.c | 4 ++-- >> hw/ide/ahci.h | 5 +++++ >> hw/ide/cmd646.c | 8 ++++---- >> hw/ide/ich.c | 10 +++++----- >> hw/ide/piix.c | 8 ++++---- >> hw/ide/via.c | 4 ++-- >> hw/isa/i82378.c | 8 ++++---- >> hw/isa/lpc_ich9.c | 6 +++--- >> hw/misc/ivshmem.c | 18 +++++++++++------- >> hw/misc/pci-testdev.c | 11 ++++++++--- >> hw/misc/vfio.c | 4 ++-- >> hw/net/e1000.c | 18 ++++++++++++------ >> hw/net/eepro100.c | 14 ++++++++------ >> hw/net/ne2000.c | 6 ++++-- >> hw/net/pcnet-pci.c | 14 +++++++++----- >> hw/net/rtl8139.c | 26 ++++++++++++++++++-------- >> hw/pci-bridge/dec.c | 2 +- >> hw/pci-bridge/i82801b11.c | 2 +- >> hw/pci-bridge/ioh3420.c | 2 +- >> hw/pci-bridge/pci_bridge_dev.c | 2 +- >> hw/pci-bridge/xio3130_downstream.c | 2 +- >> hw/pci-bridge/xio3130_upstream.c | 2 +- >> hw/pci-host/apb.c | 4 ++-- >> hw/pci-host/q35.c | 4 ++-- >> hw/pci/pci-hotplug.c | 18 ++++++++++-------- >> hw/pci/pci.c | 17 +++++++++-------- >> hw/pci/pci_bridge.c | 7 ++++--- >> hw/pci/pcie.c | 4 ++-- >> hw/pci/shpc.c | 8 ++++---- >> hw/scsi/esp-pci.c | 14 +++++++++----- >> hw/scsi/lsi53c895a.c | 26 ++++++++++++++++---------- >> hw/scsi/megasas.c | 15 ++++++++++----- >> hw/scsi/vmw_pvscsi.c | 2 +- >> hw/usb/hcd-ehci-pci.c | 13 ++++++++----- >> hw/usb/hcd-ohci.c | 2 +- >> hw/usb/hcd-uhci.c | 2 +- >> hw/usb/hcd-xhci.c | 19 +++++++++++++------ >> hw/watchdog/wdt_i6300esb.c | 2 +- >> hw/xen/xen_platform.c | 28 ++++++++++++++++------------ >> 45 files changed, 258 insertions(+), 177 deletions(-) >> > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
