Re: [Qemu-devel] [RFC 00/34] QOM realize, device-only plus ISA conversion

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

Ping,

Am 26.11.2012 01:12, schrieb Andreas Färber:
> Hello Anthony and Paolo,
> 
> As announced at KVM Forum, I have been preparing a new approach to incrementally
> get us Anthony's QOM realizefn concept. A previous attempt by Paolo and me had
> been turned down for making this available at Object-level and over questions
> whether BlockDriverState may need its own three-stage realization model.
> 
> So here's an all-new patchset doing it at DeviceState-level only, adapting the
> signature to void realize(DeviceState *, Error **).
> CPUState is on a good way to get derived from DeviceState, so in the future
> will benefit from this approach as well.
> 
> I've picked ISADevice as an example to showcase what semantic effects the
> switch to QOM realizefn has (in the hopes the number of devices would be small):
> As requested by Anthony for QOM CPUState reset and as seen with virtual methods
> in object-oriented programming languages, it becomes the derived method's
> responsibility to decide when and whether to call the parent class' method. In
> lack of real vtables this requires to save the parent's method in case we want
> to call it; classes and matching macros may need to be added for that.
> Another point to note is that we should carefully distinguish what goes into
> the qdev initfn / QOM realizefn and what can already go into a QOM initfn.
> 
> This series is rebased onto Julien's ioport cleanups (touching on ISA devices).
> 
> It starts by preparing the "realized" property, wrapping qdev's initfn (04).
> This means setting realized to true will not yet affect its children, as seen
> in Paolo's previous patches. That can be implemented later when the realizefns
> have been reviewed not to create new devices that would mess with recursive
> child realization. (In the previous series we had recursive realization but
> on my request dropped the hook-up of qdev due to the aforementioned quirks.)
> 
> At that point there is a coexistence of QOM device realizefn and qdev initfn.
> For the first time now I have set out to actually eliminate some qdev initfns,
> that's what I chose ISA for. This consists of three parts, introducing
> realizefns for ISADevices (28) and recursively for PITs (31) and for PICs (34).
> As seen for the PCI host bridge series, I've extracted general QOM cleanups
> from the main conversion patch to arrive at a clean QOM'ish state while
> hopefully keeping the main patches readable.
> 
> This series also highlights an interesting find: Beyond the to-be-solved
> CPUState, there is also a "realize" function for BusState (02), which is not
> derived from DeviceState. :-)
> With the device-centric approach taken here it would still be possible to add
> "realized" properties to other types using their own infrastructure (e.g., a
> hardcoded setter rather than a realizefn hook).
> 
> Posted as an RFC to encourage bikeshedding, in particular about the type names
> and macros introduced. Adding new header files to move them out of the source
> files for, e.g., vl.c is left for a follow-up, but for instance I was unsure
> about TYPE_ISA_FDC (should this be TYPE_ISA_FLOPPY_DRIVE_CONTROLLER as with
> PCI_HOST_BRIDGE rather than PHB?), and naming of type names and functions is
> highly inconsistent (e.g., isa_vga vs. vga_isa, or pic vs. i8259).

So far I've heard no comments on the main QOM/ISA part of the series...

Anthony, did I understand you correctly wrt the method overriding?
(PIC/PIT) Or did you have this differently in mind?

Anyone any suggestions for improvement? If not, I'll send out a v2 this
week, with qdev_free() either rebased onto Paolo or removal dropped.
qbus was not further touched in the series, so I'd probably defer the
qbus_realize() change.

Branch below is rebased to match v1.3.0 + memory-ioport pull request.

Regards,
Andreas

> 
> Available for viewing/testing at:
> https://github.com/afaerber/qemu-cpu/commits/realize-qdev
> git://github.com/afaerber/qemu-cpu.git realize-qdev
[...]
> Andreas Färber (34):
>   qdev: Eliminate qdev_free() in favor of QOM

(depends on what we want to replace qdev_free() with)

>   qbus: QOM'ify qbus_realize()

(unrelated since qbus did not get a "realized" property in this series)

>   qdev: Fold state enum into bool realized
>   qdev: Prepare "realized" property
>   isa: Split off instance_init for ISADevice
>   applesmc: QOM'ify
>   cirrus_vga: QOM'ify ISA Cirrus VGA
>   debugcon: QOM'ify ISA debug console
>   fdc: QOM'ify ISA floppy controller
>   i82374: QOM'ify

>   i8259: Fix PIC_COMMON() macro

(applied already)

>   i8259: QOM cleanups
>   ide: QOM'ify ISA IDE
>   m48t59: QOM'ify ISA M48T59 NVRAM
>   mc146818rtc: QOM'ify
>   ne2000-isa: QOM'ify
>   parallel: QOM'ify
>   pc: QOM'ify port 92
>   pckbd: QOM'ify
>   pcspk: QOM'ify
>   sb16: QOM'ify
>   serial: QOM'ify ISA serial
>   sga: QOM'ify
>   vga-isa: QOM'ify ISA VGA
>   vmmouse: QOM'ify
>   vmport: QOM'ify
>   wdt_ib700: QOM'ify
>   isa: Use realizefn for ISADevice
>   i8254: QOM'ify
>   kvm/i8254: QOM'ify some more
>   i8254: Convert PITCommonState to QOM realizefn
>   i8259: QOM'ify some more
>   kvm/i8259: QOM'ify some more
>   i8259: Convert PICCommonState to use QOM realizefn
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg