Re: [Qemu-devel] [PATCH v2 00/30] PCI: Cleanup legacy casts in device land -- ANN: qom-next revived

["Andreas Färber" <afaerber@suse.de>]

Hi Peter,

Am 24.06.2013 08:49, schrieb peter.crosthwaite@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwaite@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
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.

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. :)

>   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?

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