Re: [PATCH RESEND v3 56/58] qdev: Convert bus-less devices to qdev_realize() with Coccinelle

[Philippe Mathieu-Daudé]

Hi Markus, Peter.

On 6/10/20 7:32 AM, Markus Armbruster wrote:
> All remaining conversions to qdev_realize() are for bus-less devices.
> Coccinelle script:
> 
>     // only correct for bus-less @dev!
> 
>     @@
>     expression errp;
>     expression dev;
>     @@
>     -    qdev_init_nofail(dev);
>     +    qdev_realize(dev, NULL, &error_fatal);
> 
>     @ depends on !(file in "hw/core/qdev.c") && !(file in "hw/core/bus.c")@
>     expression errp;
>     expression dev;
>     symbol true;
>     @@
>     -    object_property_set_bool(OBJECT(dev), true, "realized", errp);
>     +    qdev_realize(DEVICE(dev), NULL, errp);
> 
>     @ depends on !(file in "hw/core/qdev.c") && !(file in "hw/core/bus.c")@
>     expression errp;
>     expression dev;
>     symbol true;
>     @@
>     -    object_property_set_bool(dev, true, "realized", errp);
>     +    qdev_realize(DEVICE(dev), NULL, errp);

Finally. Now my ealier suggestion is easier to explain:
Rename qdev_realize() -> sysbus_realize(), extracting the qdev_realize()
part. qdev_realize() doesn't take a Bus argument anymore.
Left for later.

> 
> Note that Coccinelle chokes on ARMSSE typedef vs. macro in
> hw/arm/armsse.c.  Worked around by temporarily renaming the macro for
> the spatch run.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/arm/allwinner-a10.c                   |  2 +-
>  hw/arm/allwinner-h3.c                    |  2 +-
>  hw/arm/armsse.c                          | 20 ++++++---------
>  hw/arm/armv7m.c                          |  2 +-
>  hw/arm/aspeed.c                          |  3 +--
>  hw/arm/aspeed_ast2600.c                  |  2 +-
>  hw/arm/aspeed_soc.c                      |  2 +-
>  hw/arm/bcm2836.c                         |  3 +--
>  hw/arm/cubieboard.c                      |  2 +-
>  hw/arm/digic.c                           |  2 +-
>  hw/arm/digic_boards.c                    |  2 +-
>  hw/arm/exynos4210.c                      |  4 +--
>  hw/arm/fsl-imx25.c                       |  2 +-
>  hw/arm/fsl-imx31.c                       |  2 +-
>  hw/arm/fsl-imx6.c                        |  2 +-
>  hw/arm/fsl-imx6ul.c                      |  3 +--
>  hw/arm/fsl-imx7.c                        |  2 +-
>  hw/arm/highbank.c                        |  2 +-
>  hw/arm/imx25_pdk.c                       |  2 +-
>  hw/arm/integratorcp.c                    |  2 +-
>  hw/arm/kzm.c                             |  2 +-
>  hw/arm/mcimx6ul-evk.c                    |  2 +-
>  hw/arm/mcimx7d-sabre.c                   |  2 +-
>  hw/arm/mps2-tz.c                         |  9 +++----
>  hw/arm/mps2.c                            |  7 +++---
>  hw/arm/musca.c                           |  6 ++---
>  hw/arm/orangepi.c                        |  2 +-
>  hw/arm/raspi.c                           |  2 +-
>  hw/arm/realview.c                        |  2 +-
>  hw/arm/sabrelite.c                       |  2 +-
>  hw/arm/sbsa-ref.c                        |  2 +-
>  hw/arm/stm32f205_soc.c                   |  2 +-
>  hw/arm/stm32f405_soc.c                   |  2 +-
>  hw/arm/versatilepb.c                     |  2 +-
>  hw/arm/vexpress.c                        |  2 +-
>  hw/arm/virt.c                            |  2 +-
>  hw/arm/xilinx_zynq.c                     |  2 +-
>  hw/arm/xlnx-versal.c                     |  2 +-
>  hw/arm/xlnx-zcu102.c                     |  2 +-
>  hw/arm/xlnx-zynqmp.c                     | 10 +++-----

Peter you might want to skim at the changes (other
ARM devices out of hw/arm/ involved) but to resume
basically these types are not SysBusDev:

- cpu
- soc / container
- or-gate / irq-splitter

I reviewed all of them.

Next is for Markus.

>  hw/char/serial-isa.c                     |  2 +-
>  hw/char/serial-pci-multi.c               |  2 +-
>  hw/char/serial-pci.c                     |  2 +-
>  hw/char/serial.c                         |  4 +--

I need to review again hw/char/serial-isa.c, it is
not clear why it is a container and not a SysBusDevice.

>  hw/ide/microdrive.c                      |  3 ++-

I never had to look at the PCMCIA devices, they seem
an unfinished attempt to plug a the devices on a bus.
Maybe it is finished, but the code is not clear (and
not documented). I need more time to review.

>  hw/intc/pnv_xive.c                       |  4 +--
>  hw/intc/spapr_xive.c                     |  4 +--
>  hw/intc/xics.c                           |  2 +-
>  hw/intc/xive.c                           |  2 +-
>  hw/pci-host/pnv_phb3.c                   |  6 ++---
>  hw/pci-host/pnv_phb4.c                   |  2 +-
>  hw/pci-host/pnv_phb4_pec.c               |  2 +-
>  hw/pci-host/prep.c                       |  3 +--
>  hw/ppc/pnv.c                             | 32 ++++++++++--------------
>  hw/ppc/pnv_bmc.c                         |  2 +-
>  hw/ppc/pnv_core.c                        |  2 +-
>  hw/ppc/pnv_psi.c                         |  4 +--
>  hw/ppc/spapr.c                           |  5 ++--
>  hw/ppc/spapr_cpu_core.c                  |  2 +-
>  hw/ppc/spapr_drc.c                       |  2 +-
>  hw/ppc/spapr_iommu.c                     |  2 +-
>  hw/ppc/spapr_irq.c                       |  2 +-

Wow, lot of QOM code for PPC hardware. Not all clear
yet, in particular the pci-host devices. Apparently
a LPC bus in the middle. Need a bit more time.

>  hw/s390x/s390-skeys.c                    |  2 +-
>  hw/s390x/s390-stattrib.c                 |  2 +-
>  hw/s390x/s390-virtio-ccw.c               |  4 +--
>  hw/s390x/sclp.c                          |  2 +-
>  hw/s390x/tod.c                           |  2 +-

Eh, odd s390x stuff. No clue, I might review it too.

>  target/i386/cpu.c                        |  3 +--

What is this APIC stuff doing burried with TCG?...

All the rest that is elided and not commented is reviewed.

When do you plan to send a pullreq?

Regards,

Phil.