All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse
@ 2015-04-01 15:03 Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 1/5] hw: Mark devices picking up block backends actively FIXME Markus Armbruster
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, peter.maydell

Drives defined with if!=none, character devices defined with -serial
and -parallel, network devices defined with -net nic are all for board
initialization to wire up.  Board code calls drive_get() or similar to
find them, and creates devices with their qdev properties set
accordingly.

Except a few devices go on fishing expeditions on their own instead of
exposing a drive property for board code to set.

We can't fix this in time for the release, so do the next best thing:
contain the mistakes as far as possible so they don't become ABI:

* Mark them all with suitable FIXME comments [PATCH 1-3].

* sdhci-pci is new, set cannot_instantiate_with_device_add_yet to make
  it unavailable with -device [PATCH 4].

* A few more aren't currently available with -device, set
  cannot_instantiate_with_device_add_yet to ensure they stay
  unavailable [PATCH 5].

* Left alone: m25p80-generic and its derivatives[*], ssi-sd, pc87312.
  I suspect these can't be used sanely with -device / device_add, and
  cannot_instantiate_with_device_add_yet for them would be just fine,
  but I feel we're too close to the release to argue this now.

The following changes since commit b8a86c4ac4d04c106ba38fbd707041cba334a155:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-04-01 11:31:31 +0100)

are available in the git repository at:


  git://repo.or.cz/qemu/armbru.git tags/pull-hw-2015-04-01

for you to fetch changes up to 7157bc9cf431e1debd0afb8e9cb6ab597ef93828:

  sysbus: Make devices picking up backends unavailable with -device (2015-04-01 16:08:53 +0200)

----------------------------------------------------------------
hw: Contain drive, serial, parallel, net misuse

----------------------------------------------------------------
Markus Armbruster (5):
  hw: Mark devices picking up block backends actively FIXME
  hw: Mark devices picking up char backends actively FIXME
  hw: Mark device misusing nd_table[] FIXME
  sdhci: Make device "sdhci-pci" unavailable with -device
  sysbus: Make devices picking up backends unavailable with -device

 hw/arm/allwinner-a10.c    | 2 ++
 hw/arm/spitz.c            | 3 +++
 hw/block/m25p80.c         | 1 +
 hw/char/cadence_uart.c    | 3 +++
 hw/char/digic-uart.c      | 3 +++
 hw/char/etraxfs_ser.c     | 3 +++
 hw/char/lm32_juart.c      | 3 +++
 hw/char/lm32_uart.c       | 3 +++
 hw/char/milkymist-uart.c  | 3 +++
 hw/char/pl011.c           | 3 +++
 hw/char/stm32f2xx_usart.c | 3 +++
 hw/char/xilinx_uartlite.c | 3 +++
 hw/isa/pc87312.c          | 4 ++++
 hw/sd/milkymist-memcard.c | 3 +++
 hw/sd/pl181.c             | 3 +++
 hw/sd/sdhci.c             | 5 +++++
 hw/sd/ssi-sd.c            | 1 +
 17 files changed, 49 insertions(+)

-- 
1.9.3

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

* [Qemu-devel] [PULL 1/5] hw: Mark devices picking up block backends actively FIXME
  2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
@ 2015-04-01 15:03 ` Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 2/5] hw: Mark devices picking up char " Markus Armbruster
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, Peter Crosthwaite, Andreas Färber,
	Michael Walle, pbonzini

Drives defined with if!=none are for board initialization to wire up.
Board code calls drive_get() or similar to find them, and creates
devices with their qdev drive properties set accordingly.

Except a few devices go on a fishing expedition for a suitable backend
instead of exposing a drive property for board code to set: they call
driver_get() or drive_get_next() in their realize() or init() method
to implicitly connect to the "next" backend with a certain interface
type.

Picking up backends that way works when the devices are created by
board code.  But it's inappropriate for -device or device_add.  Not
only is this inconsistent with how the other block device models work
(they connect to a backend explicitly identified by a "drive"
property), it breaks when the "next" backend has been picked up by the
board already.

Example:

    $ qemu-system-arm -S -M connex -pflash flash.img -device ssi-sd
    Aborted (core dumped)

Mark them with suitable FIXME comments.

Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: "Andreas Färber" <andreas.faerber@web.de>
Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/spitz.c            | 1 +
 hw/block/m25p80.c         | 1 +
 hw/isa/pc87312.c          | 2 ++
 hw/sd/milkymist-memcard.c | 1 +
 hw/sd/pl181.c             | 1 +
 hw/sd/sdhci.c             | 1 +
 hw/sd/ssi-sd.c            | 1 +
 7 files changed, 8 insertions(+)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index a16831c..da02932 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -168,6 +168,7 @@ static int sl_nand_init(SysBusDevice *dev)
     DriveInfo *nand;
 
     s->ctl = 0;
+    /* FIXME use a qdev drive property instead of drive_get() */
     nand = drive_get(IF_MTD, 0, 0);
     s->nand = nand_init(nand ? blk_by_legacy_dinfo(nand) : NULL,
                         s->manf_id, s->chip_id);
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index ff1106b..afe243b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -623,6 +623,7 @@ static int m25p80_init(SSISlave *ss)
     s->dirty_page = -1;
     s->storage = blk_blockalign(s->blk, s->size);
 
+    /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_MTD);
 
     if (dinfo) {
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 40a1106..2849e8d 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -319,11 +319,13 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
         d = DEVICE(isa);
         qdev_prop_set_uint32(d, "iobase", get_fdc_iobase(s));
         qdev_prop_set_uint32(d, "irq", 6);
+        /* FIXME use a qdev drive property instead of drive_get() */
         drive = drive_get(IF_FLOPPY, 0, 0);
         if (drive != NULL) {
             qdev_prop_set_drive_nofail(d, "driveA",
                                        blk_by_legacy_dinfo(drive));
         }
+        /* FIXME use a qdev drive property instead of drive_get() */
         drive = drive_get(IF_FLOPPY, 0, 1);
         if (drive != NULL) {
             qdev_prop_set_drive_nofail(d, "driveB",
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 9661eaf..0cc53d9 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -255,6 +255,7 @@ static int milkymist_memcard_init(SysBusDevice *dev)
     DriveInfo *dinfo;
     BlockBackend *blk;
 
+    /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
     blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
     s->card = sd_init(blk, false);
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index e704b6e..bf37da6 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -490,6 +490,7 @@ static int pl181_init(SysBusDevice *sbd)
     sysbus_init_irq(sbd, &s->irq[0]);
     sysbus_init_irq(sbd, &s->irq[1]);
     qdev_init_gpio_out(dev, s->cardstatus, 2);
+    /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
     s->card = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, false);
     if (s->card == NULL) {
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 27b914a..f056c52 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1146,6 +1146,7 @@ static void sdhci_initfn(SDHCIState *s)
 {
     DriveInfo *di;
 
+    /* FIXME use a qdev drive property instead of drive_get_next() */
     di = drive_get_next(IF_SD);
     s->card = sd_init(di ? blk_by_legacy_dinfo(di) : NULL, false);
     if (s->card == NULL) {
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index a71fbca..e4b2d4f 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -255,6 +255,7 @@ static int ssi_sd_init(SSISlave *d)
     DriveInfo *dinfo;
 
     s->mode = SSI_SD_CMD;
+    /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
     s->sd = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, true);
     if (s->sd == NULL) {
-- 
1.9.3

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

* [Qemu-devel] [PULL 2/5] hw: Mark devices picking up char backends actively FIXME
  2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 1/5] hw: Mark devices picking up block backends actively FIXME Markus Armbruster
@ 2015-04-01 15:03 ` Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 3/5] hw: Mark device misusing nd_table[] FIXME Markus Armbruster
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, Peter Crosthwaite, Andreas Färber,
	Michael Walle, Antony Pavlov, Edgar E. Iglesias, pbonzini,
	Li Guang

Character devices defined with -serial and -parallel are for board
initialization to wire up.  Board code examines serial_hds[] and
parallel_hds[] to find them, and creates devices with their qdev
chardev properties set accordingly.

Except a few devices go on a fishing expedition for a suitable backend
instead of exposing a chardev property for board code to set: they use
serial_hds[] (often via qemu_char_get_next_serial()) or parallel_hds[]
in their realize() or init() method to connect to a backend.

Picking up backends that way works when the devices are created by
board code.  But it's inappropriate for -device or device_add.  Not
only is it inconsistent with how the other characrer device models
work (they connect to a backend explicitly identified by a "chardev"
property), it breaks when the backend has been picked up by the board
or a previous -device / device_add already.

Example:

    $ qemu-system-ppc64 -M bamboo -S -device i82378 -device pc87312 -device pc87312
    qemu-system-ppc64: -device pc87312: Property 'isa-parallel.chardev' can't take value 'parallel0', it's in use

Mark them with suitable FIXME comments.

Cc: Li Guang <lig.fnst@cn.fujitsu.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Antony Pavlov <antonynpavlov@gmail.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: "Andreas Färber" <andreas.faerber@web.de>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/allwinner-a10.c    | 1 +
 hw/char/cadence_uart.c    | 1 +
 hw/char/digic-uart.c      | 1 +
 hw/char/etraxfs_ser.c     | 1 +
 hw/char/lm32_juart.c      | 1 +
 hw/char/lm32_uart.c       | 1 +
 hw/char/milkymist-uart.c  | 1 +
 hw/char/pl011.c           | 1 +
 hw/char/stm32f2xx_usart.c | 1 +
 hw/char/xilinx_uartlite.c | 1 +
 hw/isa/pc87312.c          | 2 ++
 11 files changed, 12 insertions(+)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 01206f2..86965a7 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -92,6 +92,7 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(sysbusdev, 0, AW_A10_EMAC_BASE);
     sysbus_connect_irq(sysbusdev, 0, s->irq[55]);
 
+    /* FIXME use a qdev chardev prop instead of serial_hds[] */
     serial_mm_init(get_system_memory(), AW_A10_UART0_REG_BASE, 2, s->irq[1],
                    115200, serial_hds[0], DEVICE_NATIVE_ENDIAN);
 }
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index a5dc2a4..4f38f0c 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -483,6 +483,7 @@ static void cadence_uart_realize(DeviceState *dev, Error **errp)
     s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                           fifo_trigger_update, s);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
 
     if (s->chr) {
diff --git a/hw/char/digic-uart.c b/hw/char/digic-uart.c
index 8abe944..f272491 100644
--- a/hw/char/digic-uart.c
+++ b/hw/char/digic-uart.c
@@ -143,6 +143,7 @@ static void digic_uart_realize(DeviceState *dev, Error **errp)
 {
     DigicUartState *s = DIGIC_UART(dev);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
diff --git a/hw/char/etraxfs_ser.c b/hw/char/etraxfs_ser.c
index 460094e..3381b86 100644
--- a/hw/char/etraxfs_ser.c
+++ b/hw/char/etraxfs_ser.c
@@ -219,6 +219,7 @@ static int etraxfs_ser_init(SysBusDevice *dev)
                           "etraxfs-serial", R_MAX * 4);
     sysbus_init_mmio(dev, &s->mmio);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
         qemu_chr_add_handlers(s->chr,
diff --git a/hw/char/lm32_juart.c b/hw/char/lm32_juart.c
index 628a86f..03285da 100644
--- a/hw/char/lm32_juart.c
+++ b/hw/char/lm32_juart.c
@@ -117,6 +117,7 @@ static int lm32_juart_init(SysBusDevice *dev)
 {
     LM32JuartState *s = LM32_JUART(dev);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, juart_can_rx, juart_rx, juart_event, s);
diff --git a/hw/char/lm32_uart.c b/hw/char/lm32_uart.c
index 4f20966..39cad49 100644
--- a/hw/char/lm32_uart.c
+++ b/hw/char/lm32_uart.c
@@ -258,6 +258,7 @@ static int lm32_uart_init(SysBusDevice *dev)
                           "uart", R_MAX * 4);
     sysbus_init_mmio(dev, &s->iomem);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
diff --git a/hw/char/milkymist-uart.c b/hw/char/milkymist-uart.c
index d05b825..b79ea09 100644
--- a/hw/char/milkymist-uart.c
+++ b/hw/char/milkymist-uart.c
@@ -199,6 +199,7 @@ static void milkymist_uart_realize(DeviceState *dev, Error **errp)
 {
     MilkymistUartState *s = MILKYMIST_UART(dev);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 0a45115..d1cf00f 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -293,6 +293,7 @@ static void pl011_realize(DeviceState *dev, Error **errp)
 {
     PL011State *s = PL011(dev);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
 
     if (s->chr) {
diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c
index 260b053..669c4e3 100644
--- a/hw/char/stm32f2xx_usart.c
+++ b/hw/char/stm32f2xx_usart.c
@@ -198,6 +198,7 @@ static void stm32f2xx_usart_init(Object *obj)
                           TYPE_STM32F2XX_USART, 0x2000);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
 
     if (s->chr) {
diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index f7c3cae..cb54684 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -205,6 +205,7 @@ static void xilinx_uartlite_realize(DeviceState *dev, Error **errp)
 {
     XilinxUARTLite *s = XILINX_UARTLITE(dev);
 
+    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
     s->chr = qemu_char_get_next_serial();
     if (s->chr)
         qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 2849e8d..3b1fcec 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -278,6 +278,7 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
     pc87312_hard_reset(s);
 
     if (is_parallel_enabled(s)) {
+        /* FIXME use a qdev chardev prop instead of parallel_hds[] */
         chr = parallel_hds[0];
         if (chr == NULL) {
             chr = qemu_chr_new("par0", "null", NULL);
@@ -296,6 +297,7 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
 
     for (i = 0; i < 2; i++) {
         if (is_uart_enabled(s, i)) {
+            /* FIXME use a qdev chardev prop instead of serial_hds[] */
             chr = serial_hds[i];
             if (chr == NULL) {
                 snprintf(name, sizeof(name), "ser%d", i);
-- 
1.9.3

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

* [Qemu-devel] [PULL 3/5] hw: Mark device misusing nd_table[] FIXME
  2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 1/5] hw: Mark devices picking up block backends actively FIXME Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 2/5] hw: Mark devices picking up char " Markus Armbruster
@ 2015-04-01 15:03 ` Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 4/5] sdhci: Make device "sdhci-pci" unavailable with -device Markus Armbruster
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Li Guang, peter.maydell

NICs defined with -net nic are for board initialization to wire up.
Board code examines nd_table[] to find them, and creates devices with
their qdev NIC properties set accordingly.

Except "allwinner-a10" goes on a fishing expedition for NIC
configuration instead of exposing the usual NIC properties for board
code to set: it uses nd_table[0] in its instance_init() method.

Picking up the first -net nic option's configuration that way works
when the device is created by board code.  But it's inappropriate for
-device and device_add.  Not only is it inconsistent with how the
other block device models work (they get their configuration from
properties "mac", "vlan", "netdev"), it breaks when nd_table[0] has
been picked up by the board or a previous -device / device_add
already.

Example:

    $ qemu-system-arm -S -M cubieboard -device allwinner-a10
    qemu-system-arm: -device allwinner-a10: Property 'allwinner-emac.netdev' can't take value 'hub0port0', it's in use
    Aborted (core dumped)

It also breaks in other entertaining ways:

    $ qemu-system-arm -M highbank -device allwinner-a10
    qemu-system-arm: -device allwinner-a10: Unsupported NIC model: xgmac
    $ qemu-system-arm -M highbank -net nic,model=allwinner-emac -device allwinner-a10
    qemu-system-arm: Unsupported NIC model: allwinner-emac

Mark the mistake with a FIXME comment.

Cc: Li Guang <lig.fnst@cn.fujitsu.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/allwinner-a10.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 86965a7..ff249af 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -34,6 +34,7 @@ static void aw_a10_init(Object *obj)
 
     object_initialize(&s->emac, sizeof(s->emac), TYPE_AW_EMAC);
     qdev_set_parent_bus(DEVICE(&s->emac), sysbus_get_default());
+    /* FIXME use qdev NIC properties instead of nd_table[] */
     if (nd_table[0].used) {
         qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC);
         qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
-- 
1.9.3

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

* [Qemu-devel] [PULL 4/5] sdhci: Make device "sdhci-pci" unavailable with -device
  2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
                   ` (2 preceding siblings ...)
  2015-04-01 15:03 ` [Qemu-devel] [PULL 3/5] hw: Mark device misusing nd_table[] FIXME Markus Armbruster
@ 2015-04-01 15:03 ` Markus Armbruster
  2015-04-01 15:03 ` [Qemu-devel] [PULL 5/5] sysbus: Make devices picking up backends " Markus Armbruster
  2015-04-01 16:18 ` [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Peter Maydell
  5 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, peter.maydell

Device models aren't supposed to go on fishing expeditions for
backends.  They should expose suitable properties for the user to set.
For onboard devices, board code sets them.

"sdhci-pci" picks up its block backend in its realize() method with
drive_get_next() instead.  Already marked FIXME.  See the commit that
added the FIXME for a more detailed explanation of what's wrong.

We can't fix this in time for the release, but since the device is new
in 2.3, we can set cannot_instantiate_with_device_add_yet to disable
it before this mistake becomes ABI, and we have to support command
lines like

    $ qemu -drive if=sd -drive if=sd,file=sd.img -device sdhci-pci -device sdhci-pci

forever.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/sd/sdhci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index f056c52..ab13505 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1254,6 +1254,8 @@ static void sdhci_pci_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->vmsd = &sdhci_vmstate;
     dc->props = sdhci_properties;
+    /* Reason: realize() method uses drive_get_next() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sdhci_pci_info = {
-- 
1.9.3

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

* [Qemu-devel] [PULL 5/5] sysbus: Make devices picking up backends unavailable with -device
  2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
                   ` (3 preceding siblings ...)
  2015-04-01 15:03 ` [Qemu-devel] [PULL 4/5] sdhci: Make device "sdhci-pci" unavailable with -device Markus Armbruster
@ 2015-04-01 15:03 ` Markus Armbruster
  2015-04-01 16:18 ` [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Peter Maydell
  5 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, Peter Crosthwaite, Michael Walle, Antony Pavlov,
	pbonzini, Edgar E. Iglesias

Device models aren't supposed to go on fishing expeditions for
backends.  They should expose suitable properties for the user to set.
For onboard devices, board code sets them.

A number of sysbus devices pick up block backends in their init() /
instance_init() methods with drive_get_next() instead: sl-nand,
milkymist-memcard, pl181, generic-sdhci.

Likewise, a number of sysbus devices pick up character backends in
their init() / realize() methods with qemu_char_get_next_serial():
cadence_uart, digic-uart, etraxfs,serial, lm32-juart, lm32-uart,
milkymist-uart, pl011, stm32f2xx-usart, xlnx.xps-uartlite.

All these mistakes are already marked FIXME.  See the commit that
added these FIXMEs for a more detailed explanation of what's wrong.

Fortunately, only machines ppce500 and pseries-* support -device with
sysbus devices, and none of the devices above is supported with these
machines.

Set cannot_instantiate_with_device_add_yet to preserve our luck.

Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Antony Pavlov <antonynpavlov@gmail.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/spitz.c            | 2 ++
 hw/char/cadence_uart.c    | 2 ++
 hw/char/digic-uart.c      | 2 ++
 hw/char/etraxfs_ser.c     | 2 ++
 hw/char/lm32_juart.c      | 2 ++
 hw/char/lm32_uart.c       | 2 ++
 hw/char/milkymist-uart.c  | 2 ++
 hw/char/pl011.c           | 2 ++
 hw/char/stm32f2xx_usart.c | 2 ++
 hw/char/xilinx_uartlite.c | 2 ++
 hw/sd/milkymist-memcard.c | 2 ++
 hw/sd/pl181.c             | 2 ++
 hw/sd/sdhci.c             | 2 ++
 13 files changed, 26 insertions(+)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index da02932..5bf032a 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -1036,6 +1036,8 @@ static void sl_nand_class_init(ObjectClass *klass, void *data)
     k->init = sl_nand_init;
     dc->vmsd = &vmstate_sl_nand_info;
     dc->props = sl_nand_properties;
+    /* Reason: init() method uses drive_get() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sl_nand_info = {
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 4f38f0c..d145378 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -537,6 +537,8 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data)
     dc->realize = cadence_uart_realize;
     dc->vmsd = &vmstate_cadence_uart;
     dc->reset = cadence_uart_reset;
+    /* Reason: realize() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo cadence_uart_info = {
diff --git a/hw/char/digic-uart.c b/hw/char/digic-uart.c
index f272491..6d44576 100644
--- a/hw/char/digic-uart.c
+++ b/hw/char/digic-uart.c
@@ -177,6 +177,8 @@ static void digic_uart_class_init(ObjectClass *klass, void *data)
     dc->realize = digic_uart_realize;
     dc->reset = digic_uart_reset;
     dc->vmsd = &vmstate_digic_uart;
+    /* Reason: realize() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo digic_uart_info = {
diff --git a/hw/char/etraxfs_ser.c b/hw/char/etraxfs_ser.c
index 3381b86..857c136 100644
--- a/hw/char/etraxfs_ser.c
+++ b/hw/char/etraxfs_ser.c
@@ -236,6 +236,8 @@ static void etraxfs_ser_class_init(ObjectClass *klass, void *data)
 
     k->init = etraxfs_ser_init;
     dc->reset = etraxfs_ser_reset;
+    /* Reason: init() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo etraxfs_ser_info = {
diff --git a/hw/char/lm32_juart.c b/hw/char/lm32_juart.c
index 03285da..62763f2 100644
--- a/hw/char/lm32_juart.c
+++ b/hw/char/lm32_juart.c
@@ -145,6 +145,8 @@ static void lm32_juart_class_init(ObjectClass *klass, void *data)
     k->init = lm32_juart_init;
     dc->reset = juart_reset;
     dc->vmsd = &vmstate_lm32_juart;
+    /* Reason: init() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo lm32_juart_info = {
diff --git a/hw/char/lm32_uart.c b/hw/char/lm32_uart.c
index 39cad49..837a46e 100644
--- a/hw/char/lm32_uart.c
+++ b/hw/char/lm32_uart.c
@@ -285,6 +285,8 @@ static void lm32_uart_class_init(ObjectClass *klass, void *data)
     k->init = lm32_uart_init;
     dc->reset = uart_reset;
     dc->vmsd = &vmstate_lm32_uart;
+    /* Reason: init() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo lm32_uart_info = {
diff --git a/hw/char/milkymist-uart.c b/hw/char/milkymist-uart.c
index b79ea09..9b89b7e 100644
--- a/hw/char/milkymist-uart.c
+++ b/hw/char/milkymist-uart.c
@@ -235,6 +235,8 @@ static void milkymist_uart_class_init(ObjectClass *klass, void *data)
     dc->realize = milkymist_uart_realize;
     dc->reset = milkymist_uart_reset;
     dc->vmsd = &vmstate_milkymist_uart;
+    /* Reason: realize() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo milkymist_uart_info = {
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index d1cf00f..eac6fac 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -308,6 +308,8 @@ static void pl011_class_init(ObjectClass *oc, void *data)
 
     dc->realize = pl011_realize;
     dc->vmsd = &vmstate_pl011;
+    /* Reason: realize() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo pl011_arm_info = {
diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c
index 669c4e3..c9d3a1b 100644
--- a/hw/char/stm32f2xx_usart.c
+++ b/hw/char/stm32f2xx_usart.c
@@ -212,6 +212,8 @@ static void stm32f2xx_usart_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->reset = stm32f2xx_usart_reset;
+    /* Reason: instance_init() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo stm32f2xx_usart_info = {
diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index cb54684..ef883a8 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -228,6 +228,8 @@ static void xilinx_uartlite_class_init(ObjectClass *klass, void *data)
 
     dc->reset = xilinx_uartlite_reset;
     dc->realize = xilinx_uartlite_realize;
+    /* Reason: realize() method uses qemu_char_get_next_serial() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo xilinx_uartlite_info = {
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 0cc53d9..2209ef1 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -297,6 +297,8 @@ static void milkymist_memcard_class_init(ObjectClass *klass, void *data)
     k->init = milkymist_memcard_init;
     dc->reset = milkymist_memcard_reset;
     dc->vmsd = &vmstate_milkymist_memcard;
+    /* Reason: init() method uses drive_get_next() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo milkymist_memcard_info = {
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index bf37da6..11fcd47 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -508,6 +508,8 @@ static void pl181_class_init(ObjectClass *klass, void *data)
     sdc->init = pl181_init;
     k->vmsd = &vmstate_pl181;
     k->reset = pl181_reset;
+    /* Reason: init() method uses drive_get_next() */
+    k->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo pl181_info = {
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ab13505..e63367b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1297,6 +1297,8 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &sdhci_vmstate;
     dc->props = sdhci_properties;
     dc->realize = sdhci_sysbus_realize;
+    /* Reason: instance_init() method uses drive_get_next() */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sdhci_sysbus_info = {
-- 
1.9.3

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

* Re: [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse
  2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
                   ` (4 preceding siblings ...)
  2015-04-01 15:03 ` [Qemu-devel] [PULL 5/5] sysbus: Make devices picking up backends " Markus Armbruster
@ 2015-04-01 16:18 ` Peter Maydell
  2015-04-02 13:25   ` Markus Armbruster
  5 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2015-04-01 16:18 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, QEMU Developers

On 1 April 2015 at 16:03, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit b8a86c4ac4d04c106ba38fbd707041cba334a155:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-04-01 11:31:31 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-hw-2015-04-01
>
> for you to fetch changes up to 7157bc9cf431e1debd0afb8e9cb6ab597ef93828:
>
>   sysbus: Make devices picking up backends unavailable with -device (2015-04-01 16:08:53 +0200)
>
> ----------------------------------------------------------------
> hw: Contain drive, serial, parallel, net misuse

I'm afraid I can't apply this -- none of the patches have any
signed-off-by lines. (The versions on the list do, but not
the ones tagged in the git repo -- you should probably fix
your workflow so it doesn't cause you to send patches to the
list which aren't the same as the ones in the pull request.)

thanks
-- PMM

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

* Re: [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse
  2015-04-01 16:18 ` [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Peter Maydell
@ 2015-04-02 13:25   ` Markus Armbruster
  0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2015-04-02 13:25 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers

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

> On 1 April 2015 at 16:03, Markus Armbruster <armbru@redhat.com> wrote:
>> The following changes since commit b8a86c4ac4d04c106ba38fbd707041cba334a155:
>>
>>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream'
>> into staging (2015-04-01 11:31:31 +0100)
>>
>> are available in the git repository at:
>>
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-hw-2015-04-01
>>
>> for you to fetch changes up to 7157bc9cf431e1debd0afb8e9cb6ab597ef93828:
>>
>>   sysbus: Make devices picking up backends unavailable with -device
>> (2015-04-01 16:08:53 +0200)
>>
>> ----------------------------------------------------------------
>> hw: Contain drive, serial, parallel, net misuse
>
> I'm afraid I can't apply this -- none of the patches have any
> signed-off-by lines. (The versions on the list do, but not
> the ones tagged in the git repo -- you should probably fix
> your workflow so it doesn't cause you to send patches to the
> list which aren't the same as the ones in the pull request.)

Let me try again.  Sorry for wasting your time.

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

end of thread, other threads:[~2015-04-02 13:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-01 15:03 [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Markus Armbruster
2015-04-01 15:03 ` [Qemu-devel] [PULL 1/5] hw: Mark devices picking up block backends actively FIXME Markus Armbruster
2015-04-01 15:03 ` [Qemu-devel] [PULL 2/5] hw: Mark devices picking up char " Markus Armbruster
2015-04-01 15:03 ` [Qemu-devel] [PULL 3/5] hw: Mark device misusing nd_table[] FIXME Markus Armbruster
2015-04-01 15:03 ` [Qemu-devel] [PULL 4/5] sdhci: Make device "sdhci-pci" unavailable with -device Markus Armbruster
2015-04-01 15:03 ` [Qemu-devel] [PULL 5/5] sysbus: Make devices picking up backends " Markus Armbruster
2015-04-01 16:18 ` [Qemu-devel] [PULL 0/5] hw: Contain drive, serial, parallel, net misuse Peter Maydell
2015-04-02 13:25   ` 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.