qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/24] Memory patches, part 4: region ownership (devices part)
@ 2013-06-25 14:14 Paolo Bonzini
  2013-06-25 14:14 ` [Qemu-devel] [PATCH 01/24] escc: rename struct to ESCCState Paolo Bonzini
                   ` (23 more replies)
  0 siblings, 24 replies; 31+ messages in thread
From: Paolo Bonzini @ 2013-06-25 14:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, alex.williamson

These series are a supplement to the ones I sent earlier.  They add an
owner to all memory regions that belong to a qdev-ified device.  These
patches are *not* automatically generated!  (The bulk change in this
morning's patches was generated by sed, instead).

The ownership API is easier to use than in my previous attempt; this
series is the price to pay.

Patch 1 is trivial; patches 2-11 add an owner argument, whenever needed,
to functions that create memory regions and do not already take an object.

The last 12 patches are the ones that are difficult to review.  I
reviewed them myself as follows, starting from a "ddd" file that has
the 12 patches squashed.

Due to additional dependencies on Jan's patches, it's probably more
practical if you just start from the iommu branch of my github repo.

1) Use

    grep memory_region_init ddd|grep -v ,.*,

which gave results in four files:

    hw/block/pflash_cfi01.c
    hw/i386/kvm/pci-assign.c
    hw/misc/vfio.c
    hw/pci/pci.c

reviewed those manually and dropped the hunks from the "ddd" file.

2) Run this script

    awk '{
      if (/diff/) { file = $3; sub(/^a\//, "", file) }
      else if (/^@@/) { f = $3; gensub(/^+(.*),.*/, "\\1", f); line = f - 1 }
      if (/^[ -+]/) line++;
      if (/^\+.*memory_region/) {
         sub(/\(/, " (", $2); $0 = $0;
         print file, line, $2, $4
      }
   }' ddd | grep -v OBJECT

The output lines are in the format (file, line, function called, second
argument)

   hw/alpha/typhoon.c 753 TyphoonState s

(file, line, type, variable name).  This pass gave results for these files:

    hw/acpi/core.c (uses memory_region_owner)
    hw/cpu/icc_bus.c (receives an Object *)
    hw/dma/xilinx_axidma.c (receives an Object *)
    hw/ide/macio.c (receives an Object *)
    hw/input/pckbd.c (arithmetic in the first argument, 2 occurrences)
    hw/isa/pc87312.c (receives an Object *)

Reviewed acpi/core.c and input/pckbd.c manually, removed them from the
"ddd" file.  Checked that the fourth argument is always of the shape
"OBJECT(foo)".

3) put this in get_var_type.awk

    BEGIN {
      var_re = "([A-Za-z_][A-Za-z_0-9]*)\\s*\\*\\s*" var "\\>\\s*[,=;)]"
    }
    match($0, var_re, subs) {
      type = subs[1]
    }
    NR == line {
      print FILENAME, line, type, var
    }

and run this script, similar to the one in step 2:

    awk '{
      if (/diff/) { file = $3; sub(/^a\//, "", file) }
      else if (/^@@/) { f = $3; gensub(/^+(.*),.*/, "\\1", f); line = f - 1 }
      if (/^[ -+]/) line++;
      if (/^\+.*memory_region/) {
         sub(/\(/, " (", $2); $0 = $0;
         sub(/^OBJECT\(/, "", $4)
         sub(/,$/, "", $4)
         sub(/\)$/, "", $4)
         print "awk -fget_var_type.awk", "-vline="line, "-vvar="$4, file
      }
   }' ddd | sh > fff

The "fff" file has output like this:

   hw/alpha/typhoon.c 753 TyphoonState s

(file, line, type, variable name)


4) Run

    awk '{print $3}' fff|sort -u|tee ggg

and eyeballed it for weird things.

5) Reviewed the output of this:

   for i in `cat ggg` ; do
      git grep -A1 -E '^(typedef )?struct '$i || echo "$i ???"
      echo --
   done

... checking files by hand if the definition of the struct was not
obvious from the "git grep" output.

This found 4 bugs passing as the owner NE2000State, AHCIDevice,
I82378State and DebugconState.  It also found one false positive
(SerialState for hw/char/escc.c).  All of these are of course gone in
this submission.  After finding these bugs, I redid all the steps.
This time, before step 5 I limited the scope using

   for i in `awk '{print $3}' fff|sort -u` ; do
      git grep --quiet DEFINE_PROP_.*'\b'$i'\b' || echo $i
   done > ggg

after checking that it would not cause any false negative.

Paolo

Paolo Bonzini (24):
  escc: rename struct to ESCCState
  vga: pass owner to vga_init
  vga: pass owner to vga_common_init
  vga: pass owner to cirrus_init_common
  vga: pass owner to vga_init_vbe
  vga: pass owner to vga_init_io
  vga: set owner in vga_update_memory_access
  ne2000: pass device to ne2000_setup_io, use it as owner
  vfio: pass device to vfio_mmap_bar and use it to set owner
  spapr_iommu: pass device to spapr_tce_new_table and use it to set owner
  pam: pass device to init_pam and use it to set owner
  piolist: add owner argument to initialization functions and pass devices
  hw/a*: pass owner to memory_region_init_io
  hw/block: pass owner to memory_region_init_io
  hw/c*: pass owner to memory_region_init_io
  hw/d*: pass owner to memory_region_init_io
  hw/gpio: pass owner to memory_region_init_io
  hw/i*: pass owner to memory_region_init_io
  hw/m*: pass owner to memory_region_init_io
  hw/n*: pass owner to memory_region_init_io
  hw/p*: pass owner to memory_region_init_io
  hw/s*: pass owner to memory_region_init_io
  hw/t*: pass owner to memory_region_init_io
  hw/[u-x]*: pass owner to memory_region_init_io

 hw/acpi/core.c                 |  9 ++++++---
 hw/acpi/ich9.c                 | 10 +++++-----
 hw/acpi/piix4.c                | 14 +++++++-------
 hw/alpha/typhoon.c             | 21 ++++++++++++---------
 hw/arm/armv7m.c                |  4 ++--
 hw/arm/highbank.c              |  4 ++--
 hw/arm/integratorcp.c          |  7 ++++---
 hw/arm/musicpal.c              | 20 ++++++++++----------
 hw/arm/pxa2xx.c                | 10 ++++++----
 hw/arm/pxa2xx_gpio.c           |  2 +-
 hw/arm/pxa2xx_pic.c            |  2 +-
 hw/arm/spitz.c                 |  2 +-
 hw/arm/stellaris.c             |  6 +++---
 hw/arm/strongarm.c             | 18 ++++++++++++------
 hw/arm/versatilepb.c           |  3 ++-
 hw/audio/ac97.c                |  6 ++++--
 hw/audio/adlib.c               |  2 +-
 hw/audio/cs4231.c              |  3 ++-
 hw/audio/cs4231a.c             |  3 ++-
 hw/audio/es1370.c              |  2 +-
 hw/audio/intel-hda.c           |  2 +-
 hw/audio/marvell_88w8618.c     |  2 +-
 hw/audio/milkymist-ac97.c      |  2 +-
 hw/audio/pcspk.c               |  2 +-
 hw/audio/pl041.c               |  2 +-
 hw/block/fdc.c                 |  7 ++++---
 hw/block/nvme.c                |  3 ++-
 hw/block/onenand.c             | 10 ++++++----
 hw/block/pflash_cfi01.c        |  2 +-
 hw/block/pflash_cfi02.c        |  8 ++++----
 hw/char/cadence_uart.c         |  2 +-
 hw/char/debugcon.c             |  2 +-
 hw/char/escc.c                 | 36 ++++++++++++++++++------------------
 hw/char/etraxfs_ser.c          |  3 ++-
 hw/char/exynos4210_uart.c      |  4 ++--
 hw/char/grlib_apbuart.c        |  2 +-
 hw/char/imx_serial.c           |  3 ++-
 hw/char/lm32_uart.c            |  3 ++-
 hw/char/milkymist-uart.c       |  2 +-
 hw/char/pl011.c                |  2 +-
 hw/char/serial-isa.c           |  2 +-
 hw/char/serial-pci.c           |  7 ++++---
 hw/char/tpci200.c              | 12 ++++++------
 hw/char/xilinx_uartlite.c      |  4 ++--
 hw/core/empty_slot.c           |  2 +-
 hw/cpu/a15mpcore.c             |  3 ++-
 hw/cpu/a9mpcore.c              |  2 +-
 hw/cpu/arm11mpcore.c           |  6 ++++--
 hw/cpu/icc_bus.c               |  2 +-
 hw/display/cirrus_vga.c        | 28 +++++++++++++++-------------
 hw/display/exynos4210_fimd.c   |  2 +-
 hw/display/jazz_led.c          |  2 +-
 hw/display/milkymist-tmu2.c    |  2 +-
 hw/display/milkymist-vgafb.c   |  2 +-
 hw/display/pl110.c             |  2 +-
 hw/display/qxl.c               | 23 ++++++++++++++---------
 hw/display/tcx.c               | 18 ++++++++++--------
 hw/display/vga-isa-mm.c        |  4 ++--
 hw/display/vga-isa.c           |  6 +++---
 hw/display/vga-pci.c           |  7 ++++---
 hw/display/vga.c               | 23 ++++++++++++-----------
 hw/display/vga_int.h           |  8 ++++----
 hw/display/vmware_vga.c        |  6 +++---
 hw/dma/i82374.c                |  2 +-
 hw/dma/pl080.c                 |  2 +-
 hw/dma/pl330.c                 |  3 ++-
 hw/dma/puv3_dma.c              |  2 +-
 hw/dma/pxa2xx_dma.c            |  2 +-
 hw/dma/sparc32_dma.c           |  3 ++-
 hw/dma/sun4m_iommu.c           |  2 +-
 hw/dma/xilinx_axidma.c         |  2 +-
 hw/gpio/omap_gpio.c            |  6 +++---
 hw/gpio/pl061.c                |  2 +-
 hw/gpio/puv3_gpio.c            |  2 +-
 hw/gpio/zaurus.c               |  2 +-
 hw/i2c/bitbang_i2c.c           |  2 +-
 hw/i2c/exynos4210_i2c.c        |  4 ++--
 hw/i2c/omap_i2c.c              |  2 +-
 hw/i2c/pm_smbus.c              |  3 ++-
 hw/i2c/versatile_i2c.c         |  2 +-
 hw/i386/kvm/pci-assign.c       | 28 +++++++++++++++-------------
 hw/i386/kvmvapic.c             |  6 +++---
 hw/i386/pc.c                   |  2 +-
 hw/ide/ahci.c                  |  6 ++++--
 hw/ide/cmd646.c                | 13 ++++++++-----
 hw/ide/macio.c                 |  2 +-
 hw/ide/mmio.c                  |  4 ++--
 hw/ide/piix.c                  |  8 ++++----
 hw/ide/via.c                   |  8 ++++----
 hw/input/milkymist-softusb.c   |  6 +++---
 hw/input/pckbd.c               |  6 ++++--
 hw/input/pl050.c               |  2 +-
 hw/intc/apic.c                 |  2 +-
 hw/intc/arm_gic.c              |  9 +++++----
 hw/intc/arm_gic_kvm.c          |  6 ++++--
 hw/intc/armv7m_nvic.c          |  7 ++++---
 hw/intc/etraxfs_pic.c          |  3 ++-
 hw/intc/exynos4210_combiner.c  |  2 +-
 hw/intc/exynos4210_gic.c       |  8 ++++----
 hw/intc/grlib_irqmp.c          |  2 +-
 hw/intc/i8259.c                |  6 ++++--
 hw/intc/imx_avic.c             |  3 ++-
 hw/intc/ioapic.c               |  3 ++-
 hw/intc/omap_intc.c            |  4 ++--
 hw/intc/openpic.c              |  6 +++---
 hw/intc/pl190.c                |  2 +-
 hw/intc/puv3_intc.c            |  2 +-
 hw/intc/realview_gic.c         |  3 ++-
 hw/intc/slavio_intctl.c        |  5 +++--
 hw/intc/xilinx_intc.c          |  3 ++-
 hw/isa/apm.c                   |  2 +-
 hw/isa/i82378.c                |  6 ++++--
 hw/isa/isa-bus.c               |  2 +-
 hw/isa/lpc_ich9.c              |  4 ++--
 hw/isa/pc87312.c               |  2 +-
 hw/isa/vt82c686.c              |  4 ++--
 hw/mips/gt64xxx_pci.c          |  2 +-
 hw/misc/a9scu.c                |  3 ++-
 hw/misc/applesmc.c             |  4 ++--
 hw/misc/arm_l2x0.c             |  3 ++-
 hw/misc/arm_sysctl.c           |  3 ++-
 hw/misc/debugexit.c            |  2 +-
 hw/misc/eccmemctl.c            |  4 ++--
 hw/misc/exynos4210_pmu.c       |  4 ++--
 hw/misc/imx_ccm.c              |  3 ++-
 hw/misc/ivshmem.c              |  8 ++++----
 hw/misc/lm32_sys.c             |  3 ++-
 hw/misc/milkymist-hpdmc.c      |  2 +-
 hw/misc/milkymist-pfpu.c       |  2 +-
 hw/misc/mst_fpga.c             |  2 +-
 hw/misc/pc-testdev.c           |  8 ++++----
 hw/misc/pci-testdev.c          |  4 ++--
 hw/misc/puv3_pm.c              |  2 +-
 hw/misc/pvpanic.c              |  2 +-
 hw/misc/slavio_misc.c          | 16 ++++++++--------
 hw/misc/vfio.c                 | 41 +++++++++++++++++++++--------------------
 hw/misc/vmport.c               |  2 +-
 hw/misc/zynq_slcr.c            |  2 +-
 hw/net/cadence_gem.c           |  3 ++-
 hw/net/e1000.c                 |  6 +++---
 hw/net/eepro100.c              | 12 ++++++------
 hw/net/etraxfs_eth.c           |  3 ++-
 hw/net/lan9118.c               |  3 ++-
 hw/net/lance.c                 |  3 ++-
 hw/net/milkymist-minimac2.c    |  4 ++--
 hw/net/mipsnet.c               |  3 ++-
 hw/net/ne2000-isa.c            |  2 +-
 hw/net/ne2000.c                |  6 +++---
 hw/net/ne2000.h                |  2 +-
 hw/net/opencores_eth.c         |  4 ++--
 hw/net/pcnet-pci.c             |  6 +++---
 hw/net/rtl8139.c               |  6 ++++--
 hw/net/smc91c111.c             |  2 +-
 hw/net/stellaris_enet.c        |  4 ++--
 hw/net/vmxnet3.c               |  6 +++---
 hw/net/xgmac.c                 |  3 ++-
 hw/net/xilinx_axienet.c        |  2 +-
 hw/net/xilinx_ethlite.c        |  4 ++--
 hw/nvram/ds1225y.c             |  3 ++-
 hw/nvram/fw_cfg.c              |  6 +++---
 hw/nvram/mac_nvram.c           |  4 ++--
 hw/pci-bridge/dec.c            |  4 ++--
 hw/pci-bridge/pci_bridge_dev.c |  2 +-
 hw/pci-host/apb.c              | 12 ++++++------
 hw/pci-host/bonito.c           | 10 +++++-----
 hw/pci-host/grackle.c          |  8 ++++----
 hw/pci-host/pam.c              | 16 +++++++++-------
 hw/pci-host/piix.c             | 17 +++++++++--------
 hw/pci-host/ppce500.c          | 12 ++++++------
 hw/pci-host/prep.c             |  8 ++++----
 hw/pci-host/q35.c              | 14 +++++++-------
 hw/pci-host/uninorth.c         | 24 ++++++++++++------------
 hw/pci-host/versatile.c        | 16 ++++++++--------
 hw/pci/msix.c                  |  6 +++---
 hw/pci/pci.c                   |  5 +++--
 hw/pci/pci_bridge.c            | 12 ++++++------
 hw/pci/pcie_host.c             |  3 ++-
 hw/pci/shpc.c                  |  4 ++--
 hw/ppc/e500.c                  |  2 +-
 hw/ppc/mpc8544_guts.c          |  2 +-
 hw/ppc/ppc4xx_pci.c            |  8 ++++----
 hw/ppc/ppce500_spin.c          |  4 ++--
 hw/ppc/prep.c                  |  2 +-
 hw/ppc/spapr_iommu.c           |  4 ++--
 hw/ppc/spapr_pci.c             | 15 +++++++++------
 hw/ppc/spapr_vio.c             |  2 +-
 hw/scsi/esp-pci.c              |  3 ++-
 hw/scsi/esp.c                  |  4 ++--
 hw/scsi/lsi53c895a.c           |  9 ++++++---
 hw/scsi/megasas.c              |  6 +++---
 hw/scsi/vmw_pvscsi.c           |  2 +-
 hw/sd/milkymist-memcard.c      |  2 +-
 hw/sd/pl181.c                  |  2 +-
 hw/sd/sdhci.c                  |  2 +-
 hw/sh4/sh_pci.c                |  6 +++---
 hw/sparc/sun4m.c               |  9 +++++----
 hw/sparc64/sun4u.c             |  4 ++--
 hw/ssi/pl022.c                 |  2 +-
 hw/ssi/xilinx_spi.c            |  3 ++-
 hw/ssi/xilinx_spips.c          |  5 +++--
 hw/timer/arm_mptimer.c         |  4 ++--
 hw/timer/arm_timer.c           |  6 ++++--
 hw/timer/cadence_ttc.c         |  3 ++-
 hw/timer/etraxfs_timer.c       |  3 ++-
 hw/timer/exynos4210_mct.c      |  4 ++--
 hw/timer/exynos4210_pwm.c      |  4 ++--
 hw/timer/exynos4210_rtc.c      |  4 ++--
 hw/timer/grlib_gptimer.c       |  3 ++-
 hw/timer/hpet.c                |  2 +-
 hw/timer/i8254.c               |  3 ++-
 hw/timer/imx_epit.c            |  2 +-
 hw/timer/imx_gpt.c             |  2 +-
 hw/timer/lm32_timer.c          |  3 ++-
 hw/timer/m48t59.c              |  8 +++++---
 hw/timer/mc146818rtc.c         |  2 +-
 hw/timer/milkymist-sysctl.c    |  2 +-
 hw/timer/pl031.c               |  2 +-
 hw/timer/puv3_ost.c            |  2 +-
 hw/timer/pxa2xx_timer.c        |  2 +-
 hw/timer/slavio_timer.c        |  2 +-
 hw/timer/tusb6010.c            |  4 ++--
 hw/timer/xilinx_timer.c        |  2 +-
 hw/tpm/tpm_tis.c               |  3 ++-
 hw/usb/hcd-ehci-sysbus.c       |  2 +-
 hw/usb/hcd-ehci.c              |  8 ++++----
 hw/usb/hcd-ohci.c              |  3 ++-
 hw/usb/hcd-uhci.c              |  4 +++-
 hw/usb/hcd-xhci.c              | 12 ++++++------
 hw/virtio/virtio-pci.c         |  4 ++--
 hw/watchdog/wdt_i6300esb.c     |  3 ++-
 hw/watchdog/wdt_ib700.c        |  2 +-
 hw/xen/xen_apic.c              |  4 ++--
 hw/xen/xen_platform.c          |  7 ++++---
 hw/xen/xen_pt.c                |  4 ++--
 hw/xen/xen_pt_msi.c            |  3 ++-
 hw/xtensa/xtensa_lx60.c        |  2 +-
 include/exec/ioport.h          |  4 +++-
 include/hw/pci-host/pam.h      |  4 ++--
 include/hw/ppc/spapr.h         |  3 ++-
 ioport.c                       |  6 ++++--
 240 files changed, 703 insertions(+), 584 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2013-07-02  6:43 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-25 14:14 [Qemu-devel] [PATCH 00/24] Memory patches, part 4: region ownership (devices part) Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 01/24] escc: rename struct to ESCCState Paolo Bonzini
2013-06-25 15:10   ` Andreas Färber
2013-06-25 14:14 ` [Qemu-devel] [PATCH 02/24] vga: pass owner to vga_init Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 03/24] vga: pass owner to vga_common_init Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 04/24] vga: pass owner to cirrus_init_common Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 05/24] vga: pass owner to vga_init_vbe Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 06/24] vga: pass owner to vga_init_io Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 07/24] vga: set owner in vga_update_memory_access Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 08/24] ne2000: pass device to ne2000_setup_io, use it as owner Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 09/24] vfio: pass device to vfio_mmap_bar and use it to set owner Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 10/24] spapr_iommu: pass device to spapr_tce_new_table " Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 11/24] pam: pass device to init_pam " Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 12/24] piolist: add owner argument to initialization functions and pass devices Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 13/24] hw/a*: pass owner to memory_region_init_io Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 14/24] hw/block: " Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 15/24] hw/c*: " Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 16/24] hw/d*: " Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 17/24] hw/gpio: " Paolo Bonzini
2013-06-25 14:14 ` [Qemu-devel] [PATCH 18/24] hw/i*: " Paolo Bonzini
2013-07-01 23:27   ` Andreas Färber
2013-06-25 14:15 ` [Qemu-devel] [PATCH 19/24] hw/m*: " Paolo Bonzini
2013-06-25 14:15 ` [Qemu-devel] [PATCH 20/24] hw/n*: " Paolo Bonzini
2013-06-25 14:15 ` [Qemu-devel] [PATCH 21/24] hw/p*: " Paolo Bonzini
2013-07-01 23:31   ` Andreas Färber
2013-07-02  6:43     ` Paolo Bonzini
2013-06-25 14:15 ` [Qemu-devel] [PATCH 22/24] hw/s*: " Paolo Bonzini
2013-06-25 14:15 ` [Qemu-devel] [PATCH 23/24] hw/t*: " Paolo Bonzini
2013-06-25 14:15 ` [Qemu-devel] [PATCH 24/24] hw/[u-x]*: " Paolo Bonzini
2013-07-01 23:42   ` Andreas Färber
2013-07-02  5:49     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).