* [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user
@ 2013-10-29 16:08 armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet armbru
` (9 more replies)
0 siblings, 10 replies; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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.
The series makes following devices available with device_add:
* PCI [PATCH 07-08]: piix3-ide, piix3-ide-xen, piix4-ide, via-ide
* ISA [PATCH 09]: i8042, isa-fdc
The following devices are made unavailable:
* PCI [PATCH 05]: dec-21154, e500-host-bridge, gt64120_pci, mch,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, versatile_pci_host
* Sysbus [PATCH 02]: ARM,bitband-memory, SUNW,CS4231, SUNW,fdtwo,
SUNW,tcx, a15mpcore_priv, a9-scu, a9mpcore_priv, apc,
arm11mpcore_priv, cadence_gem, cadence_ttc, cadence_uart,
cfi.pflash01, cfi.pflash02, cuda, dec-21154-sysbus, ds1225y,
e500-ccsr, e500-pcihost, e500-spin, eccmemctl, empty_slot, escc,
esp, etraxfs,pic, etraxfs,serial, etraxfs,timer, etraxfs-eth,
exynos4210-ehci-usb, exynos4210.combiner, exynos4210.fimd,
exynos4210.gic, exynos4210.i2c, exynos4210.irq_gate, exynos4210.mct,
exynos4210.pmu, exynos4210.pwm, exynos4210.rtc, exynos4210.uart,
fusbh200-ehci-usb, generic-sdhci, gpio_i2c, grlib,apbuart,
grlib,gptimer, grlib,irqmp, gt64120, highbank-regs, icc-bridge,
imx-serial, imx.epit, imx.gpt, imx_avic, imx_ccm, integrator_core,
integrator_pic, integrator_pit, iommu, jazz-led, lan9118, lance,
lm32-juart, lm32-pic, lm32-sys, lm32-timer, lm32-uart, m48t59,
macio-ide, macio-nvram, macio_idreg, mainstone-fpga, memory,
milkymist-ac97, milkymist-hpdmc, milkymist-memcard,
milkymist-minimac2, milkymist-pfpu, milkymist-softusb,
milkymist-sysctl, milkymist-tmu2, milkymist-uart, milkymist-vgafb,
mips-malta, mipsnet, mmio-ide, mpc8544-guts, musicpal-misc,
musicpal_gpio, musicpal_key, musicpal_lcd, mv88w8618_audio,
mv88w8618_eth, mv88w8618_flashcfg, mv88w8618_pic, mv88w8618_pit,
mv88w8618_wlan, omap-gpio, omap-intc, omap2-gpio, omap2-intc,
omap_i2c, onenand, open_eth, openpic, openprom, pbm, pl011,
pl011_luminary, pl022, pl050_keyboard, pl050_mouse, pl061,
pl061_luminary, pl330, ppc4xx-pcihost, puv3_dma, puv3_gpio,
puv3_intc, puv3_ost, puv3_pm, pxa25x-timer, pxa27x-timer,
pxa2xx-dma, pxa2xx-gpio, pxa2xx-ssp, pxa2xx_i2c, pxa2xx_pic,
pxa2xx_rtc, q35-pcihost, realview_gic, realview_mpcore,
realview_pci, realview_sysctl, s390-sclp-event-facility, scoop,
sh_pci, sl-nand, slavio_intctl, slavio_misc, slavio_timer,
smc91c111, sp804, spapr-pci-host-bridge, sparc32_dma,
spitz-keyboard, stellaris-adc, stellaris-gptm, stellaris-i2c,
stellaris_enet, strongarm-gpio, strongarm-ppc, strongarm-rtc,
strongarm-ssp, strongarm-uart, strongarm_pic, sysbus-ahci,
sysbus-fdc, sysbus-g364, sysbus-ohci, tcx_afx, tegra2-ehci-usb,
tusb6010, u3-agp-pcihost, uni-north-agp-pcihost,
uni-north-internal-pci-pcihost, uni-north-pci-pcihost,
versatile_i2c, versatile_pci, virtio-mmio, xgmac, xics,
xilinx,zynq_slcr, xlnx,ps7-usb, xlnx.axi-dma, xlnx.ps7-qspi,
xlnx.ps7-spi, xlnx.xps-ethernetlite, xlnx.xps-intc, xlnx.xps-spi,
xlnx.xps-timer, xlnx.xps-uartlite
v2: address Peter Maydell's review
* Some commit messages improved
* Use QOM cast macros instead of .parent_class [PATCH 05]
* keep cannot_instantiate_with_device_add_yet for port92, isa-pit,
kvm-pit, m48t59_isa, mc146818rtc [PATCH 09]
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 | 7 ++++++-
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 | 6 ++++++
hw/misc/arm_l2x0.c | 1 -
hw/misc/vmport.c | 3 ++-
hw/nvram/fw_cfg.c | 1 -
hw/pci-bridge/dec.c | 6 ++++++
hw/pci-host/apb.c | 6 ++++++
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 | 24 ++++++++++++++++++++++++
hw/pci-host/versatile.c | 6 ++++++
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 | 6 ++++++
hw/timer/arm_mptimer.c | 1 -
hw/timer/hpet.c | 1 -
hw/timer/i8254_common.c | 7 ++++++-
hw/timer/m48t59.c | 3 ++-
hw/timer/mc146818rtc.c | 3 ++-
hw/timer/pl031.c | 1 -
include/hw/qdev-core.h | 13 ++++++++++++-
qdev-monitor.c | 7 ++++---
qom/cpu.c | 6 +++++-
58 files changed, 187 insertions(+), 65 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
@ 2013-10-29 16:08 ` armbru
2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet armbru
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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 2d87600..f1a0a9d 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -827,7 +827,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:08 ` armbru
2013-10-30 10:13 ` Marcel Apfelbaum
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 03/10] cpu: Document why cannot_instantiate_with_device_add_yet armbru
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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 unconnected, and could not possibly work.
Quite a few, but not all sysbus devices already 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 f1a0a9d..44ee62a 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -827,7 +827,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 03/10] cpu: Document why cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:08 ` armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 04/10] apic: " armbru
` (6 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 04/10] apic: Document why cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (2 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 03/10] cpu: Document why cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:08 ` armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet armbru
` (5 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (3 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 04/10] apic: " armbru
@ 2013-10-29 16:08 ` armbru
2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 06/10] ich9: Document why cannot_instantiate_with_device_add_yet armbru
` (4 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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, mch,
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 | 6 ++++++
hw/pci-bridge/dec.c | 6 ++++++
hw/pci-host/apb.c | 6 ++++++
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 | 24 ++++++++++++++++++++++++
hw/pci-host/versatile.c | 6 ++++++
hw/ppc/ppc4xx_pci.c | 5 +++++
hw/sh4/sh_pci.c | 6 ++++++
13 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..6398514 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
static void gt64120_pci_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = gt64120_pci_init;
k->vendor_id = PCI_VENDOR_ID_MARVELL;
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.
+ */
+ dc->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..a6ca940 100644
--- a/hw/pci-bridge/dec.c
+++ b/hw/pci-bridge/dec.c
@@ -116,6 +116,7 @@ static int dec_21154_pci_host_init(PCIDevice *d)
static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = dec_21154_pci_host_init;
k->vendor_id = PCI_VENDOR_ID_DEC;
@@ -123,6 +124,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.
+ */
+ dc->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..1b399dd 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -516,11 +516,17 @@ static int pbm_pci_host_init(PCIDevice *d)
static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = pbm_pci_host_init;
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.
+ */
+ dc->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..ae04be5 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -351,12 +351,18 @@ static int unin_internal_pci_host_init(PCIDevice *d)
static void unin_main_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = unin_main_pci_host_init;
k->vendor_id = PCI_VENDOR_ID_APPLE;
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.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo unin_main_pci_host_info = {
@@ -369,12 +375,18 @@ static const TypeInfo unin_main_pci_host_info = {
static void u3_agp_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = u3_agp_pci_host_init;
k->vendor_id = PCI_VENDOR_ID_APPLE;
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.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo u3_agp_pci_host_info = {
@@ -387,12 +399,18 @@ static const TypeInfo u3_agp_pci_host_info = {
static void unin_agp_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = unin_agp_pci_host_init;
k->vendor_id = PCI_VENDOR_ID_APPLE;
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.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo unin_agp_pci_host_info = {
@@ -405,12 +423,18 @@ static const TypeInfo unin_agp_pci_host_info = {
static void unin_internal_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = unin_internal_pci_host_init;
k->vendor_id = PCI_VENDOR_ID_APPLE;
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.
+ */
+ dc->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..71ff0de 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -467,11 +467,17 @@ static int versatile_pci_host_init(PCIDevice *d)
static void versatile_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->init = versatile_pci_host_init;
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.
+ */
+ dc->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..a2f6d9e 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -162,10 +162,16 @@ static int sh_pci_host_init(PCIDevice *d)
static void sh_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
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.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo sh_pci_host_info = {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 06/10] ich9: Document why cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (4 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:08 ` armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet armbru
` (3 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (5 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 06/10] ich9: Document why cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:08 ` armbru
2013-10-29 16:26 ` Eric Blake
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 08/10] vt82c686: " armbru
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 08/10] vt82c686: Clean up use of cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (6 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:08 ` armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 09/10] isa: " armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work armbru
9 siblings, 0 replies; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (7 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 08/10] vt82c686: " armbru
@ 2013-10-29 16:08 ` armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work armbru
9 siblings, 0 replies; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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: drop
* i8042: drop, even though its I/O base is hardcoded (because you
could conceivably still add one to a board that has none), and even
though PC board code wires up the A20 line (because that wiring is
optional)
* port92: keep because it needs additional wiring by port92_init()
* mc146818rtc: keep because it needs to be wired up by rtc_init()
* m48t59_isa: keep because needs to be wired up by m48t59_init_isa()
* isa-pit, kvm-pit: keep (in their abstract base pic-common) because
the PIT needs additional wiring by board code, depending on HPET
presence
* 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' 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 | 7 ++++++-
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 | 7 ++++++-
hw/timer/m48t59.c | 3 ++-
hw/timer/mc146818rtc.c | 3 ++-
10 files changed, 29 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..20402ba 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -544,10 +544,15 @@ 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;
+ /*
+ * Reason: unlike ordinary ISA devices, this one needs additional
+ * wiring: its A20 output line needs to be wired up by
+ * port92_init().
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo port92_info = {
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..9db5c9d 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -282,7 +282,12 @@ 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 */
+ /*
+ * Reason: unlike ordinary ISA devices, the PIT may need to be
+ * wired to the HPET, and because of that, some wiring is always
+ * done by board code.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo pit_common_type = {
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index f81cf48..8005503 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -750,9 +750,10 @@ 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;
+ /* Reason: needs to be wired up by m48t59_init_isa() */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo m48t59_isa_info = {
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 2f58220..17e1907 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -906,9 +906,10 @@ 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;
+ /* Reason: needs to be wired up by rtc_init() */
+ dc->cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo mc146818rtc_info = {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
` (8 preceding siblings ...)
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 09/10] isa: " armbru
@ 2013-10-29 16:08 ` armbru
2013-10-30 9:45 ` Marcel Apfelbaum
9 siblings, 1 reply; 25+ messages in thread
From: armbru @ 2013-10-29 16:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet armbru
@ 2013-10-29 16:26 ` Eric Blake
2013-10-29 17:37 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2013-10-29 16:26 UTC (permalink / raw)
To: armbru, qemu-devel; +Cc: kwolf, borntraeger, afaerber, anthony, peter.maydell
[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]
On 10/29/2013 10:08 AM, armbru@redhat.com wrote:
> 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.
> +++ 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;
Is it intentional that you initialize the field twice to the same value?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet
2013-10-29 16:26 ` Eric Blake
@ 2013-10-29 17:37 ` Markus Armbruster
0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2013-10-29 17:37 UTC (permalink / raw)
To: Eric Blake
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
Eric Blake <eblake@redhat.com> writes:
> On 10/29/2013 10:08 AM, armbru@redhat.com wrote:
>> 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.
>
>> +++ 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;
>
> Is it intentional that you initialize the field twice to the same value?
Nope, editing error. Will fix, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet armbru
@ 2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-30 12:21 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2013-10-30 9:45 UTC (permalink / raw)
To: armbru; +Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
On Tue, 2013-10-29 at 17:08 +0100, 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.
>
> 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 2d87600..f1a0a9d 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -827,7 +827,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");
Maybe also the message can be changed here?
> }
> error_printf("\n");
> @@ -127,7 +127,8 @@ static void qdev_print_devinfos(bool show_no_user)
Same question about show_no_user parameter, maybe give it a "better"
name?
Seems OK to me.
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
> 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 = {
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work armbru
@ 2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-30 12:15 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2013-10-30 9:45 UTC (permalink / raw)
To: armbru; +Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
On Tue, 2013-10-29 at 17:08 +0100, armbru@redhat.com wrote:
> 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;
> }
Minor comment, if you move the if statement after
k = DEVICE_CLASS(obj);
you don't need the cast anymore.
Seems OK to me.
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet armbru
@ 2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-30 12:30 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2013-10-30 9:45 UTC (permalink / raw)
To: armbru; +Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
On Tue, 2013-10-29 at 17:08 +0100, 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.
>
> 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, mch,
> 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 | 6 ++++++
> hw/pci-bridge/dec.c | 6 ++++++
> hw/pci-host/apb.c | 6 ++++++
> 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 | 24 ++++++++++++++++++++++++
> hw/pci-host/versatile.c | 6 ++++++
> hw/ppc/ppc4xx_pci.c | 5 +++++
> hw/sh4/sh_pci.c | 6 ++++++
> 13 files changed, 89 insertions(+), 4 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 3da2e67..6398514 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
> static void gt64120_pci_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = gt64120_pci_init;
> k->vendor_id = PCI_VENDOR_ID_MARVELL;
> 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.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
What do you think about a different approach: check class_id
in parent class init func and set the flag according to it?
It corresponds to your idea of changing only sysbus base class.
Here we don't have a "natural" base class, but we can use class_id.
What do you think?
Marcel
> }
>
> static const TypeInfo gt64120_pci_info = {
> diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c
> index e5e3be8..a6ca940 100644
> --- a/hw/pci-bridge/dec.c
> +++ b/hw/pci-bridge/dec.c
> @@ -116,6 +116,7 @@ static int dec_21154_pci_host_init(PCIDevice *d)
> static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = dec_21154_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_DEC;
> @@ -123,6 +124,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.
> + */
> + dc->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..1b399dd 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -516,11 +516,17 @@ static int pbm_pci_host_init(PCIDevice *d)
> static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = pbm_pci_host_init;
> 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.
> + */
> + dc->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..ae04be5 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -351,12 +351,18 @@ static int unin_internal_pci_host_init(PCIDevice *d)
> static void unin_main_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = unin_main_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> 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.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> }
>
> static const TypeInfo unin_main_pci_host_info = {
> @@ -369,12 +375,18 @@ static const TypeInfo unin_main_pci_host_info = {
> static void u3_agp_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = u3_agp_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> 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.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> }
>
> static const TypeInfo u3_agp_pci_host_info = {
> @@ -387,12 +399,18 @@ static const TypeInfo u3_agp_pci_host_info = {
> static void unin_agp_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = unin_agp_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> 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.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> }
>
> static const TypeInfo unin_agp_pci_host_info = {
> @@ -405,12 +423,18 @@ static const TypeInfo unin_agp_pci_host_info = {
> static void unin_internal_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = unin_internal_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> 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.
> + */
> + dc->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..71ff0de 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -467,11 +467,17 @@ static int versatile_pci_host_init(PCIDevice *d)
> static void versatile_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = versatile_pci_host_init;
> 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.
> + */
> + dc->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..a2f6d9e 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -162,10 +162,16 @@ static int sh_pci_host_init(PCIDevice *d)
> static void sh_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> 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.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> }
>
> static const TypeInfo sh_pci_host_info = {
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet armbru
@ 2013-10-30 10:13 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2013-10-30 10:13 UTC (permalink / raw)
To: armbru; +Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
On Tue, 2013-10-29 at 17:08 +0100, 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 unconnected, and could not possibly work.
>
> Quite a few, but not all sysbus devices already 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>
Seems OK to me.
Reviewed-by: Marcel Apfelbaum <marcel.a@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 f1a0a9d..44ee62a 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -827,7 +827,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;
> }
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
2013-10-30 9:45 ` Marcel Apfelbaum
@ 2013-10-30 12:15 ` Markus Armbruster
2013-10-30 14:20 ` Marcel Apfelbaum
0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2013-10-30 12:15 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
Marcel Apfelbaum <marcel.a@redhat.com> writes:
> On Tue, 2013-10-29 at 17:08 +0100, armbru@redhat.com wrote:
>> 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;
>> }
> Minor comment, if you move the if statement after
> k = DEVICE_CLASS(obj);
> you don't need the cast anymore.
Ignorant question: does DEVICE_CLASS(NULL) work and return NULL?
> Seems OK to me.
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
2013-10-30 9:45 ` Marcel Apfelbaum
@ 2013-10-30 12:21 ` Markus Armbruster
0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2013-10-30 12:21 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
Marcel Apfelbaum <marcel.a@redhat.com> writes:
> On Tue, 2013-10-29 at 17:08 +0100, 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.
>>
>> 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>
[...]
>> 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");
> Maybe also the message can be changed here?
I'd rather not change output of "info qdm" without good reason.
>> }
>> error_printf("\n");
>> @@ -127,7 +127,8 @@ static void qdev_print_devinfos(bool show_no_user)
> Same question about show_no_user parameter, maybe give it a "better"
> name?
My excuse for keeping show_no_user is it controls whether "no-user" is
printed.
Regardless, I'm willing to rename if folks think it's useful.
> Seems OK to me.
>
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
Thanks!
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
2013-10-30 9:45 ` Marcel Apfelbaum
@ 2013-10-30 12:30 ` Markus Armbruster
2013-10-30 13:12 ` Andreas Färber
0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2013-10-30 12:30 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
Marcel Apfelbaum <marcel.a@redhat.com> writes:
> On Tue, 2013-10-29 at 17:08 +0100, 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.
>>
>> 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, mch,
>> 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 | 6 ++++++
>> hw/pci-bridge/dec.c | 6 ++++++
>> hw/pci-host/apb.c | 6 ++++++
>> 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 | 24 ++++++++++++++++++++++++
>> hw/pci-host/versatile.c | 6 ++++++
>> hw/ppc/ppc4xx_pci.c | 5 +++++
>> hw/sh4/sh_pci.c | 6 ++++++
>> 13 files changed, 89 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index 3da2e67..6398514 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
>> static void gt64120_pci_class_init(ObjectClass *klass, void *data)
>> {
>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>
>> k->init = gt64120_pci_init;
>> k->vendor_id = PCI_VENDOR_ID_MARVELL;
>> 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.
>> + */
>> + dc->cannot_instantiate_with_device_add_yet = true;
> I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct.
> What do you think about a different approach: check class_id
> in parent class init func and set the flag according to it?
> It corresponds to your idea of changing only sysbus base class.
> Here we don't have a "natural" base class, but we can use class_id.
> What do you think?
My understanding of QOM is rather limited, so take the following with
due skepticism.
I'm afraid the parent's class_init() runs before the child's, and
therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
class_init().
To factor common initialization code, I figure I'd have to splice in an
abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
bridge types such as this one. Might make sense, but it's a bit more
than I bargained for in this series :)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
2013-10-30 12:30 ` Markus Armbruster
@ 2013-10-30 13:12 ` Andreas Färber
2013-10-30 13:54 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Andreas Färber @ 2013-10-30 13:12 UTC (permalink / raw)
To: Markus Armbruster, Marcel Apfelbaum
Cc: kwolf, borntraeger, qemu-devel, anthony, peter.maydell
Am 30.10.2013 13:30, schrieb Markus Armbruster:
> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>
>> On Tue, 2013-10-29 at 17:08 +0100, 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.
>>>
>>> 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, mch,
>>> 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 | 6 ++++++
>>> hw/pci-bridge/dec.c | 6 ++++++
>>> hw/pci-host/apb.c | 6 ++++++
>>> 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 | 24 ++++++++++++++++++++++++
>>> hw/pci-host/versatile.c | 6 ++++++
>>> hw/ppc/ppc4xx_pci.c | 5 +++++
>>> hw/sh4/sh_pci.c | 6 ++++++
>>> 13 files changed, 89 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>>> index 3da2e67..6398514 100644
>>> --- a/hw/mips/gt64xxx_pci.c
>>> +++ b/hw/mips/gt64xxx_pci.c
>>> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
>>> static void gt64120_pci_class_init(ObjectClass *klass, void *data)
>>> {
>>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>
>>> k->init = gt64120_pci_init;
>>> k->vendor_id = PCI_VENDOR_ID_MARVELL;
>>> 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.
>>> + */
>>> + dc->cannot_instantiate_with_device_add_yet = true;
>> I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
>
> Correct.
>
>> What do you think about a different approach: check class_id
>> in parent class init func and set the flag according to it?
>> It corresponds to your idea of changing only sysbus base class.
>> Here we don't have a "natural" base class, but we can use class_id.
>> What do you think?
>
> My understanding of QOM is rather limited, so take the following with
> due skepticism.
>
> I'm afraid the parent's class_init() runs before the child's, and
> therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
> class_init().
Right.
> To factor common initialization code, I figure I'd have to splice in an
> abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
> bridge types such as this one. Might make sense, but it's a bit more
> than I bargained for in this series :)
I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE
in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a
base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the
controller on the PCIBus it exposes?
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
2013-10-30 13:12 ` Andreas Färber
@ 2013-10-30 13:54 ` Markus Armbruster
2013-10-30 14:30 ` Marcel Apfelbaum
0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2013-10-30 13:54 UTC (permalink / raw)
To: Andreas Färber
Cc: kwolf, peter.maydell, Marcel Apfelbaum, qemu-devel, borntraeger, anthony
Andreas Färber <afaerber@suse.de> writes:
> Am 30.10.2013 13:30, schrieb Markus Armbruster:
>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>>
>>> On Tue, 2013-10-29 at 17:08 +0100, 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.
>>>>
>>>> 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, mch,
>>>> 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 | 6 ++++++
>>>> hw/pci-bridge/dec.c | 6 ++++++
>>>> hw/pci-host/apb.c | 6 ++++++
>>>> 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 | 24 ++++++++++++++++++++++++
>>>> hw/pci-host/versatile.c | 6 ++++++
>>>> hw/ppc/ppc4xx_pci.c | 5 +++++
>>>> hw/sh4/sh_pci.c | 6 ++++++
>>>> 13 files changed, 89 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>>>> index 3da2e67..6398514 100644
>>>> --- a/hw/mips/gt64xxx_pci.c
>>>> +++ b/hw/mips/gt64xxx_pci.c
>>>> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
>>>> static void gt64120_pci_class_init(ObjectClass *klass, void *data)
>>>> {
>>>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>>
>>>> k->init = gt64120_pci_init;
>>>> k->vendor_id = PCI_VENDOR_ID_MARVELL;
>>>> 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.
>>>> + */
>>>> + dc->cannot_instantiate_with_device_add_yet = true;
>>> I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
>>
>> Correct.
>>
>>> What do you think about a different approach: check class_id
>>> in parent class init func and set the flag according to it?
>>> It corresponds to your idea of changing only sysbus base class.
>>> Here we don't have a "natural" base class, but we can use class_id.
>>> What do you think?
>>
>> My understanding of QOM is rather limited, so take the following with
>> due skepticism.
>>
>> I'm afraid the parent's class_init() runs before the child's, and
>> therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
>> class_init().
>
> Right.
>
>> To factor common initialization code, I figure I'd have to splice in an
>> abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
>> bridge types such as this one. Might make sense, but it's a bit more
>> than I bargained for in this series :)
>
> I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE
> in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a
> base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the
> controller on the PCIBus it exposes?
Yes. Sorry for the poor choice of name; I should've checked I got a new
one.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
2013-10-30 12:15 ` Markus Armbruster
@ 2013-10-30 14:20 ` Marcel Apfelbaum
2013-10-30 14:49 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2013-10-30 14:20 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
On Wed, 2013-10-30 at 13:15 +0100, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>
> > On Tue, 2013-10-29 at 17:08 +0100, armbru@redhat.com wrote:
> >> 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;
> >> }
> > Minor comment, if you move the if statement after
> > k = DEVICE_CLASS(obj);
> > you don't need the cast anymore.
>
> Ignorant question: does DEVICE_CLASS(NULL) work and return NULL?
Checked it, yes, it will return NULL.
Marcel
>
> > Seems OK to me.
> > Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
>
> Thanks!
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
2013-10-30 13:54 ` Markus Armbruster
@ 2013-10-30 14:30 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2013-10-30 14:30 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony,
Andreas Färber
On Wed, 2013-10-30 at 14:54 +0100, Markus Armbruster wrote:
> Andreas Färber <afaerber@suse.de> writes:
>
> > Am 30.10.2013 13:30, schrieb Markus Armbruster:
> >> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> >>
> >>> On Tue, 2013-10-29 at 17:08 +0100, 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.
> >>>>
> >>>> 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, mch,
> >>>> 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 | 6 ++++++
> >>>> hw/pci-bridge/dec.c | 6 ++++++
> >>>> hw/pci-host/apb.c | 6 ++++++
> >>>> 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 | 24 ++++++++++++++++++++++++
> >>>> hw/pci-host/versatile.c | 6 ++++++
> >>>> hw/ppc/ppc4xx_pci.c | 5 +++++
> >>>> hw/sh4/sh_pci.c | 6 ++++++
> >>>> 13 files changed, 89 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> >>>> index 3da2e67..6398514 100644
> >>>> --- a/hw/mips/gt64xxx_pci.c
> >>>> +++ b/hw/mips/gt64xxx_pci.c
> >>>> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
> >>>> static void gt64120_pci_class_init(ObjectClass *klass, void *data)
> >>>> {
> >>>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >>>> + DeviceClass *dc = DEVICE_CLASS(klass);
> >>>>
> >>>> k->init = gt64120_pci_init;
> >>>> k->vendor_id = PCI_VENDOR_ID_MARVELL;
> >>>> 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.
> >>>> + */
> >>>> + dc->cannot_instantiate_with_device_add_yet = true;
> >>> I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
> >>
> >> Correct.
> >>
> >>> What do you think about a different approach: check class_id
> >>> in parent class init func and set the flag according to it?
> >>> It corresponds to your idea of changing only sysbus base class.
> >>> Here we don't have a "natural" base class, but we can use class_id.
> >>> What do you think?
> >>
> >> My understanding of QOM is rather limited, so take the following with
> >> due skepticism.
> >>
> >> I'm afraid the parent's class_init() runs before the child's, and
> >> therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
> >> class_init().
> >
> > Right.
So the only way would be a base class.
> >
> >> To factor common initialization code, I figure I'd have to splice in an
> >> abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
> >> bridge types such as this one. Might make sense, but it's a bit more
> >> than I bargained for in this series :)
Yes, I suppose I wouldn't do it either only for this flag that will eventually
disappear, so
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
> >
> > I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE
> > in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a
> > base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the
> > controller on the PCIBus it exposes?
>
> Yes. Sorry for the poor choice of name; I should've checked I got a new
> one.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
2013-10-30 14:20 ` Marcel Apfelbaum
@ 2013-10-30 14:49 ` Markus Armbruster
0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2013-10-30 14:49 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, qemu-devel, borntraeger, anthony, afaerber
Marcel Apfelbaum <marcel.a@redhat.com> writes:
> On Wed, 2013-10-30 at 13:15 +0100, Markus Armbruster wrote:
>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>>
>> > On Tue, 2013-10-29 at 17:08 +0100, armbru@redhat.com wrote:
>> >> 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;
>> >> }
>> > Minor comment, if you move the if statement after
>> > k = DEVICE_CLASS(obj);
>> > you don't need the cast anymore.
>>
>> Ignorant question: does DEVICE_CLASS(NULL) work and return NULL?
> Checked it, yes, it will return NULL.
I'll give it a try when I respin. Thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2013-10-30 14:50 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-29 16:08 [Qemu-devel] [PATCH v2 00/10] Clean up and fix no_user armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet armbru
2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-30 12:21 ` Markus Armbruster
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet armbru
2013-10-30 10:13 ` Marcel Apfelbaum
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 03/10] cpu: Document why cannot_instantiate_with_device_add_yet armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 04/10] apic: " armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet armbru
2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-30 12:30 ` Markus Armbruster
2013-10-30 13:12 ` Andreas Färber
2013-10-30 13:54 ` Markus Armbruster
2013-10-30 14:30 ` Marcel Apfelbaum
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 06/10] ich9: Document why cannot_instantiate_with_device_add_yet armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet armbru
2013-10-29 16:26 ` Eric Blake
2013-10-29 17:37 ` Markus Armbruster
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 08/10] vt82c686: " armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 09/10] isa: " armbru
2013-10-29 16:08 ` [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work armbru
2013-10-30 9:45 ` Marcel Apfelbaum
2013-10-30 12:15 ` Markus Armbruster
2013-10-30 14:20 ` Marcel Apfelbaum
2013-10-30 14:49 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.