[Qemu-devel] [PATCH 00/10] Clean up and fix no_user

[Qemu-devel] [PATCH 00/10] Clean up and fix no_user

[armbru]

From: Markus Armbruster <armbru@redhat.com>

In an ideal world, machines can be built by wiring devices together
with configuration, not code.  Unfortunately, that's not the world we
live in right now.  We still have quite a few devices that need to be
wired up by code.  If you try to device_add such a device, it'll fail
in sometimes mysterious ways.  If you're lucky, you get an
unmysterious immediate crash.

We used to protect users from such badness by marking devices where
device_add cannot possibly work "no-user", and refusing to device_add
them.  Anthony silently broke the protection in v1.1.  He has rejected
attempts to unbreak it with the argument that the protection makes it
impossible to wire devices together with configuration, not code, and
that the protection is being misused[*].

On the former, I disagree.  The problem isn't protecting users from
devices that cannot be wired up that way, it's devices that cannot be
wired up that way.

On the latter, Anthony has a point: the purpose of the no-user flag
isn't obvious, and some of its uses are suspect.

So, instead of just fixing the regression, this series first addresses
that point.  PATCH 1 clarifies the purpose of no-user.  PATCH 2-9
clean up and document its use.  PATCH 10 fixes the regression.

Markus Armbruster (10):
  qdev: Replace no_user by cannot_instantiate_with_device_add_yet
  sysbus: Set cannot_instantiate_with_device_add_yet
  cpu: Document why cannot_instantiate_with_device_add_yet
  apic: Document why cannot_instantiate_with_device_add_yet
  pci-host: Consistently set cannot_instantiate_with_device_add_yet
  ich9: Document why cannot_instantiate_with_device_add_yet
  piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet
  vt82c686: Clean up use of cannot_instantiate_with_device_add_yet
  isa: Clean up use of cannot_instantiate_with_device_add_yet
  qdev: Do not let the user try to device_add when it cannot work

 hw/acpi/piix4.c            |  7 ++++++-
 hw/alpha/typhoon.c         |  2 --
 hw/arm/versatilepb.c       |  1 -
 hw/audio/pcspk.c           |  3 ++-
 hw/audio/pl041.c           |  1 -
 hw/block/fdc.c             |  1 -
 hw/core/sysbus.c           |  7 +++++++
 hw/display/pl110.c         |  1 -
 hw/dma/pl080.c             |  1 -
 hw/i2c/smbus_ich9.c        |  6 +++++-
 hw/i386/kvm/clock.c        |  1 -
 hw/i386/kvmvapic.c         |  1 -
 hw/i386/pc.c               |  1 -
 hw/ide/piix.c              |  3 ---
 hw/ide/via.c               |  1 -
 hw/input/pckbd.c           |  1 -
 hw/input/vmmouse.c         |  3 ++-
 hw/intc/apic_common.c      |  6 +++++-
 hw/intc/arm_gic.c          |  1 -
 hw/intc/arm_gic_common.c   |  1 -
 hw/intc/arm_gic_kvm.c      |  1 -
 hw/intc/i8259_common.c     |  8 +++++++-
 hw/intc/ioapic_common.c    |  1 -
 hw/intc/pl190.c            |  1 -
 hw/isa/isa-bus.c           |  1 -
 hw/isa/lpc_ich9.c          |  7 +++++--
 hw/isa/piix4.c             |  6 +++++-
 hw/isa/vt82c686.c          |  6 +++++-
 hw/mips/gt64xxx_pci.c      |  5 +++++
 hw/misc/arm_l2x0.c         |  1 -
 hw/misc/vmport.c           |  3 ++-
 hw/nvram/fw_cfg.c          |  1 -
 hw/pci-bridge/dec.c        |  5 +++++
 hw/pci-host/apb.c          |  5 +++++
 hw/pci-host/bonito.c       |  8 +++++---
 hw/pci-host/grackle.c      |  8 +++++---
 hw/pci-host/piix.c         | 19 +++++++++++++++----
 hw/pci-host/ppce500.c      |  5 +++++
 hw/pci-host/prep.c         |  7 +++++--
 hw/pci-host/q35.c          |  5 +++++
 hw/pci-host/uninorth.c     | 20 ++++++++++++++++++++
 hw/pci-host/versatile.c    |  5 +++++
 hw/ppc/ppc4xx_pci.c        |  5 +++++
 hw/ppc/spapr_vio.c         |  2 --
 hw/s390x/ipl.c             |  1 -
 hw/s390x/s390-virtio-bus.c |  2 --
 hw/s390x/virtio-ccw.c      |  2 --
 hw/sd/pl181.c              |  1 -
 hw/sh4/sh_pci.c            |  5 +++++
 hw/timer/arm_mptimer.c     |  1 -
 hw/timer/hpet.c            |  1 -
 hw/timer/i8254_common.c    |  1 -
 hw/timer/m48t59.c          |  1 -
 hw/timer/mc146818rtc.c     |  1 -
 hw/timer/pl031.c           |  1 -
 include/hw/qdev-core.h     | 13 ++++++++++++-
 qdev-monitor.c             |  7 ++++---
 qom/cpu.c                  |  6 +++++-
 58 files changed, 162 insertions(+), 65 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

In an ideal world, machines can be built by wiring devices together
with configuration, not code.  Unfortunately, that's not the world we
live in right now.  We still have quite a few devices that need to be
wired up by code.  If you try to device_add such a device, it'll fail
in sometimes mysterious ways.  If you're lucky, you get an
unmysterious immediate crash.

To protect users from such badness, DeviceClass member no_user used to
make device models unavailable with -device / device_add, but that
regressed in commit 18b6dad.  The device model is still omitted from
help, but is available anyway.

Attempts to fix the regression have been rejected with the argument
that the purpose of no_user isn't clear, and it's prone to misuse.

This commit clarifies no_user's purpose.  Anthony suggested to rename
it cannot_instantiate_with_device_add_yet_due_to_internal_bugs, which
I shorten somewhat to keep checkpatch happy.  While there, make it
bool.

Every use of cannot_instantiate_with_device_add_yet gets a FIXME
comment asking for rationale.  The next few commits will clean them
all up, either by providing a rationale, or by getting rid of the use.

With that done, the regression fix is hopefully acceptable.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/acpi/piix4.c            |  2 +-
 hw/alpha/typhoon.c         |  2 +-
 hw/arm/versatilepb.c       |  2 +-
 hw/audio/pcspk.c           |  2 +-
 hw/audio/pl041.c           |  2 +-
 hw/block/fdc.c             |  2 +-
 hw/display/pl110.c         |  2 +-
 hw/dma/pl080.c             |  2 +-
 hw/i2c/smbus_ich9.c        |  2 +-
 hw/i386/kvm/clock.c        |  2 +-
 hw/i386/kvmvapic.c         |  2 +-
 hw/i386/pc.c               |  2 +-
 hw/ide/piix.c              |  6 +++---
 hw/ide/via.c               |  2 +-
 hw/input/pckbd.c           |  2 +-
 hw/input/vmmouse.c         |  2 +-
 hw/intc/apic_common.c      |  2 +-
 hw/intc/arm_gic.c          |  2 +-
 hw/intc/arm_gic_common.c   |  2 +-
 hw/intc/arm_gic_kvm.c      |  2 +-
 hw/intc/i8259_common.c     |  2 +-
 hw/intc/ioapic_common.c    |  2 +-
 hw/intc/pl190.c            |  2 +-
 hw/isa/isa-bus.c           |  2 +-
 hw/isa/lpc_ich9.c          |  2 +-
 hw/isa/piix4.c             |  2 +-
 hw/isa/vt82c686.c          |  2 +-
 hw/misc/arm_l2x0.c         |  2 +-
 hw/misc/vmport.c           |  2 +-
 hw/nvram/fw_cfg.c          |  2 +-
 hw/pci-host/bonito.c       |  4 ++--
 hw/pci-host/grackle.c      |  4 ++--
 hw/pci-host/piix.c         |  8 ++++----
 hw/pci-host/prep.c         |  4 ++--
 hw/ppc/spapr_vio.c         |  2 +-
 hw/s390x/ipl.c             |  2 +-
 hw/s390x/s390-virtio-bus.c |  2 +-
 hw/s390x/virtio-ccw.c      |  2 +-
 hw/sd/pl181.c              |  2 +-
 hw/timer/arm_mptimer.c     |  2 +-
 hw/timer/hpet.c            |  2 +-
 hw/timer/i8254_common.c    |  2 +-
 hw/timer/m48t59.c          |  2 +-
 hw/timer/mc146818rtc.c     |  2 +-
 hw/timer/pl031.c           |  2 +-
 include/hw/qdev-core.h     | 13 ++++++++++++-
 qdev-monitor.c             |  5 +++--
 qom/cpu.c                  |  2 +-
 48 files changed, 69 insertions(+), 57 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b46bd5e..c29a703 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -508,7 +508,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     k->revision = 0x03;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
     dc->desc = "PM";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_acpi;
     dc->props = piix4_pm_properties;
 }
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 59e1bb8..60987ed 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -938,7 +938,7 @@ static void typhoon_pcihost_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = typhoon_pcihost_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo typhoon_pcihost_info = {
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index f7e8b7e..bb0c0ba 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -390,7 +390,7 @@ static void vpb_sic_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = vpb_sic_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_vpb_sic;
 }
 
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 9004ce3..8e3e178 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -192,7 +192,7 @@ static void pcspk_class_initfn(ObjectClass *klass, void *data)
 
     dc->realize = pcspk_realizefn;
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = pcspk_properties;
 }
 
diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c
index 5393b52..8ba661a 100644
--- a/hw/audio/pl041.c
+++ b/hw/audio/pl041.c
@@ -632,7 +632,7 @@ static void pl041_device_class_init(ObjectClass *klass, void *data)
 
     k->init = pl041_init;
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = pl041_device_reset;
     dc->vmsd = &vmstate_pl041;
     dc->props = pl041_device_properties;
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index c5a6c21..86f4920 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2234,7 +2234,7 @@ static void isabus_fdc_class_init(ObjectClass *klass, void *data)
 
     dc->realize = isabus_fdc_realize;
     dc->fw_name = "fdc";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = fdctrl_external_reset_isa;
     dc->vmsd = &vmstate_isa_fdc;
     dc->props = isa_fdc_properties;
diff --git a/hw/display/pl110.c b/hw/display/pl110.c
index 790e510..7ad5972 100644
--- a/hw/display/pl110.c
+++ b/hw/display/pl110.c
@@ -496,7 +496,7 @@ static void pl110_class_init(ObjectClass *klass, void *data)
 
     k->init = pl110_initfn;
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_pl110;
 }
 
diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
index 35b9015..a515621 100644
--- a/hw/dma/pl080.c
+++ b/hw/dma/pl080.c
@@ -381,7 +381,7 @@ static void pl080_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_pl080;
 }
 
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index ca22978..c1ffa34 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -97,7 +97,7 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_ICH9_6;
     k->revision = ICH9_A2_SMB_REVISION;
     k->class_id = PCI_CLASS_SERIAL_SMBUS;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_ich9_smbus;
     dc->desc = "ICH9 SMBUS Bridge";
     k->init = ich9_smbus_initfn;
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 383938d..abd2ce8 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -114,7 +114,7 @@ static void kvmclock_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = kvmclock_realize;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &kvmclock_vmsd;
 }
 
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 1c2dbf5..8ec3151 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -824,7 +824,7 @@ static void vapic_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset   = vapic_reset;
     dc->vmsd    = &vmstate_vapic;
     dc->realize = vapic_realize;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0c313fe..fe33843 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -544,7 +544,7 @@ static void port92_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->realize = port92_realizefn;
     dc->reset = port92_reset;
     dc->vmsd = &vmstate_port92_isa;
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ab36749..27b08e1 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -248,7 +248,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo piix3_ide_info = {
@@ -267,7 +267,7 @@ static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->unplug = pci_piix3_xen_ide_unplug;
 }
 
@@ -289,7 +289,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo piix4_ide_info = {
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 99468c7..b556c14 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -225,7 +225,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
     k->revision = 0x06;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo via_ide_info = {
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index ce86237..dee31a6 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -522,7 +522,7 @@ static void i8042_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = i8042_realizefn;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_kbd_isa;
 }
 
diff --git a/hw/input/vmmouse.c b/hw/input/vmmouse.c
index abd032b..600e4a2 100644
--- a/hw/input/vmmouse.c
+++ b/hw/input/vmmouse.c
@@ -282,7 +282,7 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = vmmouse_realizefn;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = vmmouse_reset;
     dc->vmsd = &vmstate_vmmouse;
     dc->props = vmmouse_properties;
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index a0beb10..ea420c7 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -386,7 +386,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
 
     dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = apic_properties_common;
     idc->init = apic_init_common;
 }
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index d431b7a..24ad276 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -704,7 +704,7 @@ static void arm_gic_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     ARMGICClass *agc = ARM_GIC_CLASS(klass);
 
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     agc->parent_realize = dc->realize;
     dc->realize = arm_gic_realize;
 }
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 709b5c2..9047143 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -156,7 +156,7 @@ static void arm_gic_common_class_init(ObjectClass *klass, void *data)
     dc->realize = arm_gic_common_realize;
     dc->props = arm_gic_common_properties;
     dc->vmsd = &vmstate_gic;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo arm_gic_common_type = {
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index f713975..a0bbf12 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -150,7 +150,7 @@ static void kvm_arm_gic_class_init(ObjectClass *klass, void *data)
     kgc->parent_reset = dc->reset;
     dc->realize = kvm_arm_gic_realize;
     dc->reset = kvm_arm_gic_reset;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo kvm_arm_gic_info = {
diff --git a/hw/intc/i8259_common.c b/hw/intc/i8259_common.c
index 803d037..2acdbfe 100644
--- a/hw/intc/i8259_common.c
+++ b/hw/intc/i8259_common.c
@@ -135,7 +135,7 @@ static void pic_common_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_pic_common;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = pic_properties_common;
     dc->realize = pic_common_realize;
 }
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 6b705c1..cc5a80d 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -98,7 +98,7 @@ static void ioapic_common_class_init(ObjectClass *klass, void *data)
 
     dc->realize = ioapic_common_realize;
     dc->vmsd = &vmstate_ioapic_common;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo ioapic_common_type = {
diff --git a/hw/intc/pl190.c b/hw/intc/pl190.c
index 329680d..b16bc02 100644
--- a/hw/intc/pl190.c
+++ b/hw/intc/pl190.c
@@ -273,7 +273,7 @@ static void pl190_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = pl190_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = pl190_reset;
     dc->vmsd = &vmstate_pl190;
 }
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 9e104eb..6b2114d 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -197,7 +197,7 @@ static void isabus_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->fw_name = "isa";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo isabus_bridge_info = {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 5633d08..ad841b5 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -604,7 +604,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
     dc->reset = ich9_lpc_reset;
     k->init = ich9_lpc_initfn;
     dc->vmsd = &vmstate_ich9_lpc;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     k->config_write = ich9_lpc_config_write;
     dc->desc = "ICH9 LPC bridge";
     k->vendor_id = PCI_VENDOR_ID_INTEL;
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 1a1d451..d9dac61 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -113,7 +113,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
     dc->desc = "ISA bridge";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_piix4;
 }
 
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8fe4fcb..3e8ec80 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -480,7 +480,7 @@ static void via_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_BRIDGE_ISA;
     k->revision = 0x40;
     dc->desc = "ISA bridge";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_via;
 }
 
diff --git a/hw/misc/arm_l2x0.c b/hw/misc/arm_l2x0.c
index 8e192cd..ceea99d 100644
--- a/hw/misc/arm_l2x0.c
+++ b/hw/misc/arm_l2x0.c
@@ -179,7 +179,7 @@ static void l2x0_class_init(ObjectClass *klass, void *data)
 
     k->init = l2x0_priv_init;
     dc->vmsd = &vmstate_l2x0;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = l2x0_properties;
     dc->reset = l2x0_priv_reset;
 }
diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
index 0b5a564..94ae6ae 100644
--- a/hw/misc/vmport.c
+++ b/hw/misc/vmport.c
@@ -162,7 +162,7 @@ static void vmport_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = vmport_realizefn;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo vmport_info = {
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d0820e5..553599f 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -576,7 +576,7 @@ static void fw_cfg_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = fw_cfg_realize;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = fw_cfg_reset;
     dc->vmsd = &vmstate_fw_cfg;
     dc->props = fw_cfg_properties;
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 5086d42..2e08e9d 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -806,7 +806,7 @@ static void bonito_class_init(ObjectClass *klass, void *data)
     k->revision = 0x01;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "Host bridge";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_bonito;
 }
 
@@ -823,7 +823,7 @@ static void bonito_pcihost_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = bonito_pcihost_initfn;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo bonito_pcihost_info = {
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 4991ec4..a2c5f56 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -130,7 +130,7 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
     k->revision  = 0x00;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo grackle_pci_info = {
@@ -146,7 +146,7 @@ static void pci_grackle_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->init = pci_grackle_init_device;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo grackle_pci_host_info = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index c041149..697de65 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -644,7 +644,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
-    dc->no_user     = 1,
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     k->no_hotplug   = 1;
     k->init         = piix3_initfn;
     k->config_write = piix3_write_config;
@@ -668,7 +668,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
-    dc->no_user     = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     k->no_hotplug   = 1;
     k->init         = piix3_initfn;
     k->config_write = piix3_write_config_xen;
@@ -698,7 +698,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
     k->revision = 0x02;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "Host bridge";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_i440fx;
 }
 
@@ -730,7 +730,7 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
     hc->root_bus_path = i440fx_pcihost_root_bus_path;
     dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = i440fx_props;
 }
 
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 0e71fdb..58b8c5e 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -198,7 +198,7 @@ static void raven_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "PReP Host Bridge - Motorola Raven";
     dc->vmsd = &vmstate_raven;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo raven_info = {
@@ -215,7 +215,7 @@ static void raven_pcihost_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->realize = raven_pcihost_realizefn;
     dc->fw_name = "pci";
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo raven_pcihost_info = {
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index a6a0a51..1e33819 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -532,7 +532,7 @@ static void spapr_vio_bridge_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = spapr_vio_bridge_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo spapr_vio_bridge_info = {
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index d69adb2..f86a4af 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -181,7 +181,7 @@ static void s390_ipl_class_init(ObjectClass *klass, void *data)
     k->init = s390_ipl_init;
     dc->props = s390_ipl_properties;
     dc->reset = s390_ipl_reset;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo s390_ipl_info = {
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6a83111..eccc3e7 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -680,7 +680,7 @@ static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = s390_virtio_bridge_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo s390_virtio_bridge_info = {
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cd67db5..df13b70 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1278,7 +1278,7 @@ static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = virtual_css_bridge_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo virtual_css_bridge_info = {
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index c35896d..d830188 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -506,7 +506,7 @@ static void pl181_class_init(ObjectClass *klass, void *data)
     sdc->init = pl181_init;
     k->vmsd = &vmstate_pl181;
     k->reset = pl181_reset;
-    k->no_user = 1;
+    k->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo pl181_info = {
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 8020c9f..f9cdeea 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -297,7 +297,7 @@ static void arm_mptimer_class_init(ObjectClass *klass, void *data)
     sbc->init = arm_mptimer_init;
     dc->vmsd = &vmstate_arm_mptimer;
     dc->reset = arm_mptimer_reset;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = arm_mptimer_properties;
 }
 
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index fcd22ae..3777764 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -751,7 +751,7 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = hpet_realize;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = hpet_reset;
     dc->vmsd = &vmstate_hpet;
     dc->props = hpet_device_properties;
diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c
index e8fb971..dc2196c 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -282,7 +282,7 @@ static void pit_common_class_init(ObjectClass *klass, void *data)
 
     dc->realize = pit_common_realize;
     dc->vmsd = &vmstate_pit_common;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo pit_common_type = {
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index d3d78ec..f81cf48 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -750,7 +750,7 @@ static void m48t59_isa_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = m48t59_isa_realize;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = m48t59_reset_isa;
     dc->props = m48t59_isa_properties;
 }
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 7230a6e..2f58220 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -906,7 +906,7 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = rtc_realizefn;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_rtc;
     dc->props = mc146818rtc_properties;
 }
diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
index 65928a4..2f7360c 100644
--- a/hw/timer/pl031.c
+++ b/hw/timer/pl031.c
@@ -251,7 +251,7 @@ static void pl031_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = pl031_init;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_pl031;
 }
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e191ca0..2b571d7 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -97,7 +97,18 @@ typedef struct DeviceClass {
     const char *fw_name;
     const char *desc;
     Property *props;
-    int no_user;
+
+    /*
+     * Shall we hide this device model from -device / device_add?
+     * All devices should support instantiation with device_add, and
+     * this flag should not exist.  But we're not there, yet.  Some
+     * devices fail to instantiate with cryptic error messages.
+     * Others instantiate, but don't work.  Exposing users to such
+     * behavior would be cruel; this flag serves to protect them.  It
+     * should never be set without a comment explaining why it is set.
+     * TODO remove once we're there
+     */
+    bool cannot_instantiate_with_device_add_yet;
 
     /* callbacks */
     void (*reset)(DeviceState *dev);
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a02c925..36f6f09 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -87,7 +87,7 @@ static void qdev_print_devinfo(DeviceClass *dc)
     if (dc->desc) {
         error_printf(", desc \"%s\"", dc->desc);
     }
-    if (dc->no_user) {
+    if (dc->cannot_instantiate_with_device_add_yet) {
         error_printf(", no-user");
     }
     error_printf("\n");
@@ -127,7 +127,8 @@ static void qdev_print_devinfos(bool show_no_user)
             if ((i < DEVICE_CATEGORY_MAX
                  ? !test_bit(i, dc->categories)
                  : !bitmap_empty(dc->categories, DEVICE_CATEGORY_MAX))
-                || (!show_no_user && dc->no_user)) {
+                || (!show_no_user
+                    && dc->cannot_instantiate_with_device_add_yet)) {
                 continue;
             }
             if (!cat_printed) {
diff --git a/qom/cpu.c b/qom/cpu.c
index 818fb26..09c15e6 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -254,7 +254,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->gdb_read_register = cpu_common_gdb_read_register;
     k->gdb_write_register = cpu_common_gdb_write_register;
     dc->realize = cpu_common_realizefn;
-    dc->no_user = 1;
+    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo cpu_type_info = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 02/10] sysbus: Set cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

device_add plugs devices into suitable bus.  For "real" buses, that
actually connects the device.  For sysbus, the connections need to be
made separately, and device_add can't do that.  The device would be
left unconncected, and could not possibly work.

Many, but not all sysbus devices alreasy set
cannot_instantiate_with_device_add_yet in their class init function.

Set it in their abstract base's class init function
sysbus_device_class_init(), and remove the now redundant assignments
from device class init functions.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/alpha/typhoon.c         | 2 --
 hw/arm/versatilepb.c       | 1 -
 hw/audio/pl041.c           | 1 -
 hw/core/sysbus.c           | 7 +++++++
 hw/display/pl110.c         | 1 -
 hw/dma/pl080.c             | 1 -
 hw/i386/kvm/clock.c        | 1 -
 hw/i386/kvmvapic.c         | 1 -
 hw/intc/arm_gic.c          | 1 -
 hw/intc/arm_gic_common.c   | 1 -
 hw/intc/arm_gic_kvm.c      | 1 -
 hw/intc/ioapic_common.c    | 1 -
 hw/intc/pl190.c            | 1 -
 hw/isa/isa-bus.c           | 1 -
 hw/misc/arm_l2x0.c         | 1 -
 hw/nvram/fw_cfg.c          | 1 -
 hw/pci-host/bonito.c       | 2 --
 hw/pci-host/grackle.c      | 2 --
 hw/pci-host/piix.c         | 1 -
 hw/pci-host/prep.c         | 1 -
 hw/ppc/spapr_vio.c         | 2 --
 hw/s390x/ipl.c             | 1 -
 hw/s390x/s390-virtio-bus.c | 2 --
 hw/s390x/virtio-ccw.c      | 2 --
 hw/sd/pl181.c              | 1 -
 hw/timer/arm_mptimer.c     | 1 -
 hw/timer/hpet.c            | 1 -
 hw/timer/pl031.c           | 1 -
 28 files changed, 7 insertions(+), 33 deletions(-)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 60987ed..71a5a37 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -934,11 +934,9 @@ static int typhoon_pcihost_init(SysBusDevice *dev)
 
 static void typhoon_pcihost_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = typhoon_pcihost_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo typhoon_pcihost_info = {
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index bb0c0ba..aef2bde 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -390,7 +390,6 @@ static void vpb_sic_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = vpb_sic_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_vpb_sic;
 }
 
diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c
index 8ba661a..ed82be5 100644
--- a/hw/audio/pl041.c
+++ b/hw/audio/pl041.c
@@ -632,7 +632,6 @@ static void pl041_device_class_init(ObjectClass *klass, void *data)
 
     k->init = pl041_init;
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = pl041_device_reset;
     dc->vmsd = &vmstate_pl041;
     dc->props = pl041_device_properties;
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index b84cd4a..6e880a8 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -257,6 +257,13 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
+    /*
+     * device_add plugs devices into suitable bus.  For "real" buses,
+     * that actually connects the device.  For sysbus, the connections
+     * need to be made separately, and device_add can't do that.  The
+     * device would be left unconncected, and could not possibly work.
+     */
+    k->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sysbus_device_type_info = {
diff --git a/hw/display/pl110.c b/hw/display/pl110.c
index 7ad5972..ab689e9 100644
--- a/hw/display/pl110.c
+++ b/hw/display/pl110.c
@@ -496,7 +496,6 @@ static void pl110_class_init(ObjectClass *klass, void *data)
 
     k->init = pl110_initfn;
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_pl110;
 }
 
diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
index a515621..cb7bda9 100644
--- a/hw/dma/pl080.c
+++ b/hw/dma/pl080.c
@@ -381,7 +381,6 @@ static void pl080_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_pl080;
 }
 
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index abd2ce8..892aa02 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -114,7 +114,6 @@ static void kvmclock_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = kvmclock_realize;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &kvmclock_vmsd;
 }
 
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 8ec3151..5cc894a 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -824,7 +824,6 @@ static void vapic_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset   = vapic_reset;
     dc->vmsd    = &vmstate_vapic;
     dc->realize = vapic_realize;
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 24ad276..f13a927 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -704,7 +704,6 @@ static void arm_gic_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     ARMGICClass *agc = ARM_GIC_CLASS(klass);
 
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     agc->parent_realize = dc->realize;
     dc->realize = arm_gic_realize;
 }
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 9047143..84aa7fc 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -156,7 +156,6 @@ static void arm_gic_common_class_init(ObjectClass *klass, void *data)
     dc->realize = arm_gic_common_realize;
     dc->props = arm_gic_common_properties;
     dc->vmsd = &vmstate_gic;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo arm_gic_common_type = {
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index a0bbf12..59a3da5 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -150,7 +150,6 @@ static void kvm_arm_gic_class_init(ObjectClass *klass, void *data)
     kgc->parent_reset = dc->reset;
     dc->realize = kvm_arm_gic_realize;
     dc->reset = kvm_arm_gic_reset;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo kvm_arm_gic_info = {
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index cc5a80d..9ba1a26 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -98,7 +98,6 @@ static void ioapic_common_class_init(ObjectClass *klass, void *data)
 
     dc->realize = ioapic_common_realize;
     dc->vmsd = &vmstate_ioapic_common;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo ioapic_common_type = {
diff --git a/hw/intc/pl190.c b/hw/intc/pl190.c
index b16bc02..2bf359a 100644
--- a/hw/intc/pl190.c
+++ b/hw/intc/pl190.c
@@ -273,7 +273,6 @@ static void pl190_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = pl190_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = pl190_reset;
     dc->vmsd = &vmstate_pl190;
 }
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 6b2114d..55d0100 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -197,7 +197,6 @@ static void isabus_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->fw_name = "isa";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo isabus_bridge_info = {
diff --git a/hw/misc/arm_l2x0.c b/hw/misc/arm_l2x0.c
index ceea99d..9e220c9 100644
--- a/hw/misc/arm_l2x0.c
+++ b/hw/misc/arm_l2x0.c
@@ -179,7 +179,6 @@ static void l2x0_class_init(ObjectClass *klass, void *data)
 
     k->init = l2x0_priv_init;
     dc->vmsd = &vmstate_l2x0;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = l2x0_properties;
     dc->reset = l2x0_priv_reset;
 }
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 553599f..9bcfde2 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -576,7 +576,6 @@ static void fw_cfg_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = fw_cfg_realize;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = fw_cfg_reset;
     dc->vmsd = &vmstate_fw_cfg;
     dc->props = fw_cfg_properties;
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 2e08e9d..bfb9820 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -819,11 +819,9 @@ static const TypeInfo bonito_info = {
 
 static void bonito_pcihost_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = bonito_pcihost_initfn;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo bonito_pcihost_info = {
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index a2c5f56..c178375 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -143,10 +143,8 @@ static const TypeInfo grackle_pci_info = {
 static void pci_grackle_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->init = pci_grackle_init_device;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo grackle_pci_host_info = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 697de65..21ffe97 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -730,7 +730,6 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
     hc->root_bus_path = i440fx_pcihost_root_bus_path;
     dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = i440fx_props;
 }
 
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 58b8c5e..ebc40c6 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -215,7 +215,6 @@ static void raven_pcihost_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->realize = raven_pcihost_realizefn;
     dc->fw_name = "pci";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo raven_pcihost_info = {
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 1e33819..9ac15b5 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -528,11 +528,9 @@ static int spapr_vio_bridge_init(SysBusDevice *dev)
 
 static void spapr_vio_bridge_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = spapr_vio_bridge_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo spapr_vio_bridge_info = {
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index f86a4af..cc29d8e 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -181,7 +181,6 @@ static void s390_ipl_class_init(ObjectClass *klass, void *data)
     k->init = s390_ipl_init;
     dc->props = s390_ipl_properties;
     dc->reset = s390_ipl_reset;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo s390_ipl_info = {
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index eccc3e7..46c5ff1 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -676,11 +676,9 @@ static int s390_virtio_bridge_init(SysBusDevice *dev)
 
 static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = s390_virtio_bridge_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo s390_virtio_bridge_info = {
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index df13b70..71a7e66 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1274,11 +1274,9 @@ static int virtual_css_bridge_init(SysBusDevice *dev)
 
 static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = virtual_css_bridge_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo virtual_css_bridge_info = {
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index d830188..462558b 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -506,7 +506,6 @@ static void pl181_class_init(ObjectClass *klass, void *data)
     sdc->init = pl181_init;
     k->vmsd = &vmstate_pl181;
     k->reset = pl181_reset;
-    k->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo pl181_info = {
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index f9cdeea..4c59699 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -297,7 +297,6 @@ static void arm_mptimer_class_init(ObjectClass *klass, void *data)
     sbc->init = arm_mptimer_init;
     dc->vmsd = &vmstate_arm_mptimer;
     dc->reset = arm_mptimer_reset;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = arm_mptimer_properties;
 }
 
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 3777764..446a591 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -751,7 +751,6 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = hpet_realize;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = hpet_reset;
     dc->vmsd = &vmstate_hpet;
     dc->props = hpet_device_properties;
diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
index 2f7360c..34d9b44 100644
--- a/hw/timer/pl031.c
+++ b/hw/timer/pl031.c
@@ -251,7 +251,6 @@ static void pl031_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = pl031_init;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_pl031;
 }
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 03/10] cpu: Document why cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qom/cpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index 09c15e6..6e0d54e 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -254,7 +254,11 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->gdb_read_register = cpu_common_gdb_read_register;
     k->gdb_write_register = cpu_common_gdb_write_register;
     dc->realize = cpu_common_realizefn;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
+    /*
+     * Reason: CPUs still need special care by board code: wiring up
+     * IRQs, adding reset handlers, halting non-first CPUS, ...
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo cpu_type_info = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 04/10] apic: Document why cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/intc/apic_common.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index ea420c7..aaef054 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -386,9 +386,13 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
 
     dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = apic_properties_common;
     idc->init = apic_init_common;
+    /*
+     * Reason: APIC and CPU need to be wired up by
+     * x86_cpu_apic_create()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo apic_common_type = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

Many PCI host bridges consist of a sysbus device and a PCI device.
You need both for the thing to work.  Arguably, these bridges should
be modelled as a single, composite devices instead of pairs of
seemingly independent devices you can only use together, but we're not
there, yet.

Since the sysbus part can't be instantiated with device_add, yet,
permitting it with the PCI part is useless.  We shouldn't offer
useless options to the user, so let's set
cannot_instantiate_with_device_add_yet for them.

It's already set for Bonito, grackle, i440FX, and raven.  Document
why.

Set it for the others: dec-21154, e500-host-bridge, gt64120_pci,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, and versatile_pci_host.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/mips/gt64xxx_pci.c   |  5 +++++
 hw/pci-bridge/dec.c     |  5 +++++
 hw/pci-host/apb.c       |  5 +++++
 hw/pci-host/bonito.c    |  6 +++++-
 hw/pci-host/grackle.c   |  6 +++++-
 hw/pci-host/piix.c      |  6 +++++-
 hw/pci-host/ppce500.c   |  5 +++++
 hw/pci-host/prep.c      |  6 +++++-
 hw/pci-host/q35.c       |  5 +++++
 hw/pci-host/uninorth.c  | 20 ++++++++++++++++++++
 hw/pci-host/versatile.c |  5 +++++
 hw/ppc/ppc4xx_pci.c     |  5 +++++
 hw/sh4/sh_pci.c         |  5 +++++
 13 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..4756bf1 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1157,6 +1157,11 @@ static void gt64120_pci_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
     k->revision = 0x10;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo gt64120_pci_info = {
diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c
index e5e3be8..e27ecfc 100644
--- a/hw/pci-bridge/dec.c
+++ b/hw/pci-bridge/dec.c
@@ -123,6 +123,11 @@ static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
     k->revision = 0x02;
     k->class_id = PCI_CLASS_BRIDGE_PCI;
     k->is_bridge = 1;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo dec_21154_pci_host_info = {
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 92f289f..b8df0a6 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -521,6 +521,11 @@ static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_SUN;
     k->device_id = PCI_DEVICE_ID_SUN_SABRE;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo pbm_pci_host_info = {
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index bfb9820..902441f 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -806,8 +806,12 @@ static void bonito_class_init(ObjectClass *klass, void *data)
     k->revision = 0x01;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "Host bridge";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_bonito;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo bonito_info = {
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index c178375..7d95821 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -130,7 +130,11 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
     k->revision  = 0x00;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo grackle_pci_info = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 21ffe97..8089fd6 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -698,8 +698,12 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
     k->revision = 0x02;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "Host bridge";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_i440fx;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo i440fx_info = {
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index f00793d..c80b7cb 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -387,6 +387,11 @@ static void e500_host_bridge_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_MPC8533E;
     k->class_id = PCI_CLASS_PROCESSOR_POWERPC;
     dc->desc = "Host bridge";
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo e500_host_bridge_info = {
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index ebc40c6..042dc8f 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -198,7 +198,11 @@ static void raven_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "PReP Host Bridge - Motorola Raven";
     dc->vmsd = &vmstate_raven;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo raven_info = {
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ad703a4..4dd75c6 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -390,6 +390,11 @@ static void mch_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_Q35_MCH;
     k->revision = MCH_HOST_BRIDGE_REVISION_DEFAULT;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo mch_info = {
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index 91530cd..2c20d06 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -357,6 +357,11 @@ static void unin_main_pci_host_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_PCI;
     k->revision  = 0x00;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo unin_main_pci_host_info = {
@@ -375,6 +380,11 @@ static void u3_agp_pci_host_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_APPLE_U3_AGP;
     k->revision  = 0x00;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo u3_agp_pci_host_info = {
@@ -393,6 +403,11 @@ static void unin_agp_pci_host_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_AGP;
     k->revision  = 0x00;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo unin_agp_pci_host_info = {
@@ -411,6 +426,11 @@ static void unin_internal_pci_host_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_I_PCI;
     k->revision  = 0x00;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo unin_internal_pci_host_info = {
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 6b28929..de22417 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -472,6 +472,11 @@ static void versatile_pci_host_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_XILINX;
     k->device_id = PCI_DEVICE_ID_XILINX_XC2VP30;
     k->class_id = PCI_CLASS_PROCESSOR_CO;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo versatile_pci_host_info = {
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index d2d6f65..4cb7851 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -380,6 +380,11 @@ static void ppc4xx_host_bridge_class_init(ObjectClass *klass, void *data)
     k->vendor_id    = PCI_VENDOR_ID_IBM;
     k->device_id    = PCI_DEVICE_ID_IBM_440GX;
     k->class_id     = PCI_CLASS_BRIDGE_OTHER;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo ppc4xx_host_bridge_info = {
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index e81176a..72fbe61 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -166,6 +166,11 @@ static void sh_pci_host_class_init(ObjectClass *klass, void *data)
     k->init = sh_pci_host_init;
     k->vendor_id = PCI_VENDOR_ID_HITACHI;
     k->device_id = PCI_DEVICE_ID_HITACHI_SH7751R;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    k->parent_class.cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sh_pci_host_info = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 06/10] ich9: Document why cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

An ICH9 southbridge contains several PCI devices, some of them with
multiple functions.  We model each function as a separate qdev.  Two
of them need some special wiring set up in pc_q35_init() to work: the
LPC controller at 00:1f.0, and the SMBus controller at 00:1f.3.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i2c/smbus_ich9.c | 6 +++++-
 hw/isa/lpc_ich9.c   | 7 +++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index c1ffa34..8d47eaf 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -97,11 +97,15 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_ICH9_6;
     k->revision = ICH9_A2_SMB_REVISION;
     k->class_id = PCI_CLASS_SERIAL_SMBUS;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_ich9_smbus;
     dc->desc = "ICH9 SMBUS Bridge";
     k->init = ich9_smbus_initfn;
     k->config_write = ich9_smbus_write_config;
+    /*
+     * Reason: part of ICH9 southbridge, needs to be wired up by
+     * pc_q35_init()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 i2c_bus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base)
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index ad841b5..d00d698 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -604,14 +604,17 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
     dc->reset = ich9_lpc_reset;
     k->init = ich9_lpc_initfn;
     dc->vmsd = &vmstate_ich9_lpc;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     k->config_write = ich9_lpc_config_write;
     dc->desc = "ICH9 LPC bridge";
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_ICH9_8;
     k->revision = ICH9_A2_LPC_REVISION;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
-
+    /*
+     * Reason: part of ICH9 southbridge, needs to be wired up by
+     * pc_q35_init()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo ich9_lpc_info = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

A PIIX3/PIIX4 southbridge has multiple functions.  We model each
function as a separate qdev.  Two of them need some special wiring set
up in pc_init1() or mips_malta_init() to work: the ISA bridge at 01.0,
and the SMBus controller at 01.3.

The IDE controller at 01.1 (piix3-ide, piix3-ide-xen, piix4-ide) has
always had cannot_instantiate_with_device_add_yet set, but there is no
obvious reason why device_add could not work for them.  Drop it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/acpi/piix4.c    |  7 ++++++-
 hw/ide/piix.c      |  3 ---
 hw/isa/piix4.c     |  6 +++++-
 hw/pci-host/piix.c | 12 ++++++++++--
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index c29a703..c0ad010 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -508,9 +508,14 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     k->revision = 0x03;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
     dc->desc = "PM";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
+    dc->cannot_instantiate_with_device_add_yet = true;
     dc->vmsd = &vmstate_acpi;
     dc->props = piix4_pm_properties;
+    /*
+     * Reason: part of PIIX4 southbridge, needs to be wired up,
+     * e.g. by mips_malta_init()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo piix4_pm_info = {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 27b08e1..9b5960b 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -248,7 +248,6 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo piix3_ide_info = {
@@ -267,7 +266,6 @@ static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->unplug = pci_piix3_xen_ide_unplug;
 }
 
@@ -289,7 +287,6 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo piix4_ide_info = {
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index d9dac61..def6fe3 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -113,8 +113,12 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
     dc->desc = "ISA bridge";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_piix4;
+    /*
+     * Reason: part of PIIX4 southbridge, needs to be wired up,
+     * e.g. by mips_malta_init()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo piix4_info = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 8089fd6..1526fd4 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -644,7 +644,6 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     k->no_hotplug   = 1;
     k->init         = piix3_initfn;
     k->config_write = piix3_write_config;
@@ -652,6 +651,11 @@ static void piix3_class_init(ObjectClass *klass, void *data)
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id    = PCI_DEVICE_ID_INTEL_82371SB_0;
     k->class_id     = PCI_CLASS_BRIDGE_ISA;
+    /*
+     * Reason: part of PIIX3 southbridge, needs to be wired up by
+     * pc_piix.c's pc_init1()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo piix3_info = {
@@ -668,7 +672,6 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
 
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     k->no_hotplug   = 1;
     k->init         = piix3_initfn;
     k->config_write = piix3_write_config_xen;
@@ -676,6 +679,11 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id    = PCI_DEVICE_ID_INTEL_82371SB_0;
     k->class_id     = PCI_CLASS_BRIDGE_ISA;
+    /*
+     * Reason: part of PIIX3 southbridge, needs to be wired up by
+     * pc_piix.c's pc_init1()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 };
 
 static const TypeInfo piix3_xen_info = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 08/10] vt82c686: Clean up use of cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

A VT82C686B southbridge has multiple functions.  We model each
function as a separate qdev.  One of them need some special wiring set
up in mips_fulong2e_init() to work: the ISA bridge at 05.0.

The IDE controller at 05.1 (via-ide) has always had
cannot_instantiate_with_device_add_yet set, but there is no obvious
reason why device_add could not work for them.  Drop it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/via.c      | 1 -
 hw/isa/vt82c686.c | 6 +++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index b556c14..198123b 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -225,7 +225,6 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
     k->revision = 0x06;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo via_ide_info = {
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 3e8ec80..ec7c259 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -480,8 +480,12 @@ static void via_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_BRIDGE_ISA;
     k->revision = 0x40;
     dc->desc = "ISA bridge";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_via;
+    /*
+     * Reason: part of VIA VT82C686 southbridge, needs to be wired up,
+     * e.g. by mips_fulong2e_init()
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo via_info = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet

[armbru]

From: Markus Armbruster <armbru@redhat.com>

Drop it when there's no obvious reason why device_add could not work.
Else keep and document why.

* isa-fdc, port92, i8042, m48t59_isa, mc146818rtc, isa-pit, kvm-pit:
  drop (from the last two by dropping it from their abstract base
  pit-common)

* pcspk: keep because of pointer property pit, and because realize
  sets global pcspk_state

* vmmouse: keep because of pointer property ps2_mouse

* vmport: keep because realize sets global port_state

* isa-i8259, kvm-i8259: keep (in their abstract base pic-common),
  because the PICs need additional wiring: its IRQ input lines are set
  up by board code, and the wiring of the slave to the master is
  hard-coded in device model code.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/audio/pcspk.c        | 3 ++-
 hw/block/fdc.c          | 1 -
 hw/i386/pc.c            | 1 -
 hw/input/pckbd.c        | 1 -
 hw/input/vmmouse.c      | 3 ++-
 hw/intc/i8259_common.c  | 8 +++++++-
 hw/misc/vmport.c        | 3 ++-
 hw/timer/i8254_common.c | 1 -
 hw/timer/m48t59.c       | 1 -
 hw/timer/mc146818rtc.c  | 1 -
 10 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 8e3e178..f980d66 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -192,8 +192,9 @@ static void pcspk_class_initfn(ObjectClass *klass, void *data)
 
     dc->realize = pcspk_realizefn;
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = pcspk_properties;
+    /* Reason: pointer property "pit", realize sets global pcspk_state */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo pcspk_info = {
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 86f4920..592b58f 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2234,7 +2234,6 @@ static void isabus_fdc_class_init(ObjectClass *klass, void *data)
 
     dc->realize = isabus_fdc_realize;
     dc->fw_name = "fdc";
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = fdctrl_external_reset_isa;
     dc->vmsd = &vmstate_isa_fdc;
     dc->props = isa_fdc_properties;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fe33843..bebda79 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -544,7 +544,6 @@ static void port92_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->realize = port92_realizefn;
     dc->reset = port92_reset;
     dc->vmsd = &vmstate_port92_isa;
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index dee31a6..655b8c5 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -522,7 +522,6 @@ static void i8042_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = i8042_realizefn;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_kbd_isa;
 }
 
diff --git a/hw/input/vmmouse.c b/hw/input/vmmouse.c
index 600e4a2..6a50533 100644
--- a/hw/input/vmmouse.c
+++ b/hw/input/vmmouse.c
@@ -282,10 +282,11 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = vmmouse_realizefn;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = vmmouse_reset;
     dc->vmsd = &vmstate_vmmouse;
     dc->props = vmmouse_properties;
+    /* Reason: pointer property "ps2_mouse" */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo vmmouse_info = {
diff --git a/hw/intc/i8259_common.c b/hw/intc/i8259_common.c
index 2acdbfe..9d29399 100644
--- a/hw/intc/i8259_common.c
+++ b/hw/intc/i8259_common.c
@@ -135,9 +135,15 @@ static void pic_common_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_pic_common;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->props = pic_properties_common;
     dc->realize = pic_common_realize;
+    /*
+     * Reason: unlike ordinary ISA devices, the PICs need additional
+     * wiring: its IRQ input lines are set up by board code, and the
+     * wiring of the slave to the master is hard-coded in device model
+     * code.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo pic_common_type = {
diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
index 94ae6ae..cd5716a 100644
--- a/hw/misc/vmport.c
+++ b/hw/misc/vmport.c
@@ -162,7 +162,8 @@ static void vmport_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = vmport_realizefn;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
+    /* Reason: realize sets global port_state */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo vmport_info = {
diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c
index dc2196c..2236369 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -282,7 +282,6 @@ static void pit_common_class_init(ObjectClass *klass, void *data)
 
     dc->realize = pit_common_realize;
     dc->vmsd = &vmstate_pit_common;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
 }
 
 static const TypeInfo pit_common_type = {
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index f81cf48..c0dfb51 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -750,7 +750,6 @@ static void m48t59_isa_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = m48t59_isa_realize;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->reset = m48t59_reset_isa;
     dc->props = m48t59_isa_properties;
 }
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 2f58220..2faef13 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -906,7 +906,6 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = rtc_realizefn;
-    dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
     dc->vmsd = &vmstate_rtc;
     dc->props = mc146818rtc_properties;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 10/10] qdev: Do not let the user try to device_add when it cannot work

[armbru]

From: Markus Armbruster <armbru@redhat.com>

Such devices have always been unavailable and omitted from the list of
available devices shown by device_add help.  Until commit 18b6dad
silently broke the former, setting up nasty traps for unwary users,
like this one:

    $ qemu-system-x86_64 -nodefaults -monitor stdio -display none
    QEMU 1.6.50 monitor - type 'help' for more information
    (qemu) device_add apic
    Segmentation fault (core dumped)

I call that a regression.  Fix it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qdev-monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 36f6f09..c538fec 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
         }
     }
 
-    if (!obj) {
+    if (!obj || DEVICE_CLASS(obj)->cannot_instantiate_with_device_add_yet) {
         qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
         return NULL;
     }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> Many PCI host bridges consist of a sysbus device and a PCI device.
> You need both for the thing to work.  Arguably, these bridges should
> be modelled as a single, composite devices instead of pairs of
> seemingly independent devices you can only use together, but we're not
> there, yet.

I disagree here -- we should be using the modularity that our
device model provides, not arbitrarily squashing things together
into single objects just because we've foolishly exposed to the
end user direct command line access to "create any random object
whatsoever even if it doesn't make sense".

> Since the sysbus part can't be instantiated with device_add, yet,
> permitting it with the PCI part is useless.  We shouldn't offer
> useless options to the user, so let's set
> cannot_instantiate_with_device_add_yet for them.

It doesn't make sense to allow the user to create the on-PCI-bus
representation of the host controller anyway even if they could
device_add the host controller proper: creating the host controller
will always automatically create the on-PCI-bus part.

> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1157,6 +1157,11 @@ static void gt64120_pci_class_init(ObjectClass *klass, void *data)
>      k->device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
>      k->revision = 0x10;
>      k->class_id = PCI_CLASS_BRIDGE_HOST;
> +    /*
> +     * PCI-facing part of the host bridge, not usable without the
> +     * host-facing part, which can't be device_add'ed, yet.
> +     */
> +    k->parent_class.cannot_instantiate_with_device_add_yet = true;

Please don't directly access parent_class -- you should be using
the proper QOM cast macros to get the DeviceClass pointer.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 03/10] cpu: Document why cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH 04/10] apic: Document why cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH 06/10] ich9: Document why cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> An ICH9 southbridge contains several PCI devices, some of them with
> multiple functions.  We model each function as a separate qdev.  Two
> of them need some special wiring set up in pc_q35_init() to work: the
> LPC controller at 00:1f.0, and the SMBus controller at 00:1f.3.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH 08/10] vt82c686: Clean up use of cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> A VT82C686B southbridge has multiple functions.  We model each
> function as a separate qdev.  One of them need some special wiring set
> up in mips_fulong2e_init() to work: the ISA bridge at 05.0.
>
> The IDE controller at 05.1 (via-ide) has always had
> cannot_instantiate_with_device_add_yet set, but there is no obvious
> reason why device_add could not work for them.  Drop it.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:55,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> Drop it when there's no obvious reason why device_add could not work.
> Else keep and document why.
>
> * isa-fdc, port92, i8042, m48t59_isa, mc146818rtc, isa-pit, kvm-pit:
>   drop (from the last two by dropping it from their abstract base
>   pit-common)

port92 needs its a20_out qemu_irq line wiring up, doesn't it?

the pit devices have an output IRQ line that needs wiring up.

-- PMM

Re: [Qemu-devel] [PATCH 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> In an ideal world, machines can be built by wiring devices together
> with configuration, not code.  Unfortunately, that's not the world we
> live in right now.  We still have quite a few devices that need to be
> wired up by code.  If you try to device_add such a device, it'll fail
> in sometimes mysterious ways.  If you're lucky, you get an
> unmysterious immediate crash.
>
> +    /*
> +     * Shall we hide this device model from -device / device_add?
> +     * All devices should support instantiation with device_add, and
> +     * this flag should not exist.  But we're not there, yet.  Some
> +     * devices fail to instantiate with cryptic error messages.
> +     * Others instantiate, but don't work.  Exposing users to such
> +     * behavior would be cruel; this flag serves to protect them.  It
> +     * should never be set without a comment explaining why it is set.
> +     * TODO remove once we're there
> +     */
> +    bool cannot_instantiate_with_device_add_yet;

So reading this I'm still not entirely sure what the scope of this
flag is intended to be. I can think of two possibilities:

(1) the minimal definition: this device would actually crash
or cause QEMU to break if you created it with device_add
(2) a larger definition, which includes also devices which
are completely useless if created with device_add because
there's no way for the user to wire them up properly.

I think most sysbus devices are going to be in (2) but not (1),
because they should be fine to create and initialize, but they'll
just be sitting completely pointlessly totally disconnected from
the machine model.

Definition (1) is a harder boundary and more straightforward
to check against, but definition (2) is arguably a bit more useful
for the end user.

-- PMM

Re: [Qemu-devel] [PATCH 02/10] sysbus: Set cannot_instantiate_with_device_add_yet

[Peter Maydell]

On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> device_add plugs devices into suitable bus.  For "real" buses, that
> actually connects the device.  For sysbus, the connections need to be
> made separately, and device_add can't do that.  The device would be
> left unconncected, and could not possibly work.

"unconnected"

>
> Many, but not all sysbus devices alreasy set

"already"

> cannot_instantiate_with_device_add_yet in their class init function.
>
> Set it in their abstract base's class init function
> sysbus_device_class_init(), and remove the now redundant assignments
> from device class init functions.

So I think this change is probably OK (but see my comments on
patch 1 about what our definition of the flag is supposed to be).
But I'd like to see a list of the devices which this patch makes no-user
which previously weren't. Then I could eyeball the list and check
whether there's anything in it which shouldn't be.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> In an ideal world, machines can be built by wiring devices together
>> with configuration, not code.  Unfortunately, that's not the world we
>> live in right now.  We still have quite a few devices that need to be
>> wired up by code.  If you try to device_add such a device, it'll fail
>> in sometimes mysterious ways.  If you're lucky, you get an
>> unmysterious immediate crash.
>>
>> +    /*
>> +     * Shall we hide this device model from -device / device_add?
>> +     * All devices should support instantiation with device_add, and
>> +     * this flag should not exist.  But we're not there, yet.  Some
>> +     * devices fail to instantiate with cryptic error messages.
>> +     * Others instantiate, but don't work.  Exposing users to such
>> +     * behavior would be cruel; this flag serves to protect them.  It
>> +     * should never be set without a comment explaining why it is set.
>> +     * TODO remove once we're there
>> +     */
>> +    bool cannot_instantiate_with_device_add_yet;
>
> So reading this I'm still not entirely sure what the scope of this
> flag is intended to be. I can think of two possibilities:
>
> (1) the minimal definition: this device would actually crash
> or cause QEMU to break if you created it with device_add
> (2) a larger definition, which includes also devices which
> are completely useless if created with device_add because
> there's no way for the user to wire them up properly.
>
> I think most sysbus devices are going to be in (2) but not (1),
> because they should be fine to create and initialize, but they'll
> just be sitting completely pointlessly totally disconnected from
> the machine model.
>
> Definition (1) is a harder boundary and more straightforward
> to check against, but definition (2) is arguably a bit more useful
> for the end user.

I agree, and I'd like us to adopt definition (2).  I tried to express
this when I wrote "instantiate, but don't work".  Care to suggest
clearer language for this comment?

Regarding (2) being less straightforward to check against: I think we
should try hard to make our cannot_instantiate_with_device_add_yet use
correct (any device we mark that way is actually useless with
device_add), but I view completeness (all the devices that are actually
useless with -device are marked) as not quite that important.

Re: [Qemu-devel] [PATCH 02/10] sysbus: Set cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> device_add plugs devices into suitable bus.  For "real" buses, that
>> actually connects the device.  For sysbus, the connections need to be
>> made separately, and device_add can't do that.  The device would be
>> left unconncected, and could not possibly work.
>
> "unconnected"

Will fix.

>> Many, but not all sysbus devices alreasy set
>
> "already"

Will fix.

>> cannot_instantiate_with_device_add_yet in their class init function.
>>
>> Set it in their abstract base's class init function
>> sysbus_device_class_init(), and remove the now redundant assignments
>> from device class init functions.
>
> So I think this change is probably OK (but see my comments on
> patch 1 about what our definition of the flag is supposed to be).
> But I'd like to see a list of the devices which this patch makes no-user
> which previously weren't. Then I could eyeball the list and check
> whether there's anything in it which shouldn't be.

I'll include that list in v2.

Thanks!

Re: [Qemu-devel] [PATCH 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 October 2013 14:55,  <armbru@redhat.com> wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> Drop it when there's no obvious reason why device_add could not work.
>> Else keep and document why.
>>
>> * isa-fdc, port92, i8042, m48t59_isa, mc146818rtc, isa-pit, kvm-pit:
>>   drop (from the last two by dropping it from their abstract base
>>   pit-common)
>
> port92 needs its a20_out qemu_irq line wiring up, doesn't it?
>
> the pit devices have an output IRQ line that needs wiring up.

Good points; I'll have a second look at these two.  Thanks!

Re: [Qemu-devel] [PATCH 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 October 2013 14:54,  <armbru@redhat.com> wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> Many PCI host bridges consist of a sysbus device and a PCI device.
>> You need both for the thing to work.  Arguably, these bridges should
>> be modelled as a single, composite devices instead of pairs of
>> seemingly independent devices you can only use together, but we're not
>> there, yet.
>
> I disagree here -- we should be using the modularity that our
> device model provides, not arbitrarily squashing things together
> into single objects just because we've foolishly exposed to the
> end user direct command line access to "create any random object
> whatsoever even if it doesn't make sense".

I'm afraid I didn't express myself clearly.  I'm not advocating
*squashing* these components together.  I'm saying that if A and B can
only be used wired together, there should be a C composed of A, B and
the necessary wiring, and that C is what actually gets put on the board
by configuration.

>> Since the sysbus part can't be instantiated with device_add, yet,
>> permitting it with the PCI part is useless.  We shouldn't offer
>> useless options to the user, so let's set
>> cannot_instantiate_with_device_add_yet for them.
>
> It doesn't make sense to allow the user to create the on-PCI-bus
> representation of the host controller anyway even if they could
> device_add the host controller proper: creating the host controller
> will always automatically create the on-PCI-bus part.

Technically, a device_add i440FX-pcihost doesn't automatically create
i440FX *now*.

I suspect we're arguing only about what exact kind of crazy device_add
of the PCI-facing part of the PCI host bridge is.  Assuming we actually
agree it's crazy in *today's* state of things, does it matter what kind
of crazy it is?  If it doesn't matter, perhaps you could give me a hint
on how to rephrase the commit message.

>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -1157,6 +1157,11 @@ static void
>> gt64120_pci_class_init(ObjectClass *klass, void *data)
>>      k->device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
>>      k->revision = 0x10;
>>      k->class_id = PCI_CLASS_BRIDGE_HOST;
>> +    /*
>> +     * PCI-facing part of the host bridge, not usable without the
>> +     * host-facing part, which can't be device_add'ed, yet.
>> +     */
>> +    k->parent_class.cannot_instantiate_with_device_add_yet = true;
>
> Please don't directly access parent_class -- you should be using
> the proper QOM cast macros to get the DeviceClass pointer.

Will fix, thanks!