All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series.
@ 2012-10-15  8:06 Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 1/9] serial: split serial.c Gerd Hoffmann
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch series tackles serial device hotplug.

The first four patches have been on the list before, they implement
pci-serial devices featuring a hot-pluggable 16550 uart and got some
minor tweaks only.

The next two patches update the usb-serial device.  It will only show up
in the guest when the chardev is open.  You'll see the difference with
socket chardevs:  If you open the chardev (by connecting to the socket)
the device will show up in the guest, on close (disconnect) it will
disappear.

Final three patches cleanup chardev a bit and adds chardev hotplug to
the mix, which makes the other patches alot more useful.  It is the
missing bit needed to really hotplug serial devices:

   (qemu) chardev_add file,id=pciserial,path=/root/hotchardev.log
   (qemu) device_add pci-serial,id=pciserial,chardev=pciserial

And the reverse:

   (qemu) device_del pciserial
   (qemu) chardev_del pciserial

New in v2:
 - added two chardev cleanup patches.
 - switched chardev_{add,del} commands to netdev_{add,del} style.

cheers,
  Gerd

The following changes since commit 8b4a3df8081f3e6f1061ed5cbb303ad623ade66b:

  Fix popcnt in long mode (2012-10-14 14:55:09 +0400)

are available in the git repository at:
  git://git.kraxel.org/qemu serial.2

Gerd Hoffmann (9):
      serial: split serial.c
      serial: add pci variant
      serial: add windows inf file for the pci card to docs
      serial: add 2x + 4x pci variant
      usb-serial: don't magically zap chardev on umplug
      usb-serial: only expose device in guest when the chardev is open
      chardev: add error reporting for qemu_chr_new_from_opts
      chardev: fix QemuOpts lifecycle
      chardev: add hotplug support.

 default-configs/pci.mak  |    2 +
 docs/qemupciserial.inf   |  109 ++++++++++++++++++
 hmp-commands.hx          |   32 ++++++
 hmp.c                    |   23 ++++
 hmp.h                    |    2 +
 hw/Makefile.objs         |    3 +-
 hw/alpha_dp264.c         |    1 +
 hw/kzm.c                 |    2 +-
 hw/mips_fulong2e.c       |    1 +
 hw/mips_jazz.c           |    1 +
 hw/mips_malta.c          |    1 +
 hw/mips_mipssim.c        |    2 +-
 hw/mips_r4k.c            |    1 +
 hw/musicpal.c            |    2 +-
 hw/omap_uart.c           |    3 +-
 hw/openrisc_sim.c        |    3 +-
 hw/pc.c                  |    1 +
 hw/pc.h                  |   27 -----
 hw/pci_ids.h             |    1 +
 hw/petalogix_ml605_mmu.c |    2 +-
 hw/ppc/e500.c            |    2 +-
 hw/ppc405_uc.c           |    2 +-
 hw/ppc440_bamboo.c       |    2 +-
 hw/ppc_prep.c            |    1 +
 hw/pxa2xx.c              |    2 +-
 hw/serial-isa.c          |  130 ++++++++++++++++++++++
 hw/serial-pci.c          |  272 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/serial.c              |  149 ++-----------------------
 hw/serial.h              |   74 +++++++++++++
 hw/sm501.c               |    2 +-
 hw/sun4u.c               |    1 +
 hw/usb/dev-serial.c      |   21 +++-
 hw/virtex_ml507.c        |    2 +-
 hw/xtensa_lx60.c         |    3 +-
 qapi-schema.json         |   39 +++++++
 qemu-char.c              |   83 ++++++++++++---
 qemu-char.h              |    5 +-
 qmp-commands.hx          |   61 ++++++++++
 vl.c                     |    7 +-
 39 files changed, 878 insertions(+), 199 deletions(-)
 create mode 100644 docs/qemupciserial.inf
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial-pci.c
 create mode 100644 hw/serial.h

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

* [Qemu-devel] [PATCH v3 1/9] serial: split serial.c
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15 14:16   ` Anthony Liguori
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 2/9] serial: add pci variant Gerd Hoffmann
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Split serial.c into serial.c, serial.h and serial-isa.c.  While being at
creating a serial.h header file move the serial prototypes from pc.h to
the new serial.h.  The latter leads to s/pc.h/serial.h/ in tons of
boards which just want the serial bits from pc.h

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/Makefile.objs         |    2 +-
 hw/alpha_dp264.c         |    1 +
 hw/kzm.c                 |    2 +-
 hw/mips_fulong2e.c       |    1 +
 hw/mips_jazz.c           |    1 +
 hw/mips_malta.c          |    1 +
 hw/mips_mipssim.c        |    2 +-
 hw/mips_r4k.c            |    1 +
 hw/musicpal.c            |    2 +-
 hw/omap_uart.c           |    3 +-
 hw/openrisc_sim.c        |    3 +-
 hw/pc.c                  |    1 +
 hw/pc.h                  |   27 ---------
 hw/petalogix_ml605_mmu.c |    2 +-
 hw/ppc/e500.c            |    2 +-
 hw/ppc405_uc.c           |    2 +-
 hw/ppc440_bamboo.c       |    2 +-
 hw/ppc_prep.c            |    1 +
 hw/pxa2xx.c              |    2 +-
 hw/serial-isa.c          |  130 +++++++++++++++++++++++++++++++++++++++++
 hw/serial.c              |  143 ++--------------------------------------------
 hw/serial.h              |   73 +++++++++++++++++++++++
 hw/sm501.c               |    2 +-
 hw/sun4u.c               |    1 +
 hw/virtex_ml507.c        |    2 +-
 hw/xtensa_lx60.c         |    3 +-
 26 files changed, 232 insertions(+), 180 deletions(-)
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 854faa9..16e7a1e 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -20,7 +20,7 @@ common-obj-$(CONFIG_M48T59) += m48t59.o
 common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
-common-obj-$(CONFIG_SERIAL) += serial.o
+common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
index 5ea04c7..8ce04e5 100644
--- a/hw/alpha_dp264.c
+++ b/hw/alpha_dp264.c
@@ -15,6 +15,7 @@
 #include "mc146818rtc.h"
 #include "ide.h"
 #include "i8254.h"
+#include "serial.h"
 
 #define MAX_IDE_BUS 2
 
diff --git a/hw/kzm.c b/hw/kzm.c
index 68cd1b4..1f3082b 100644
--- a/hw/kzm.c
+++ b/hw/kzm.c
@@ -21,7 +21,7 @@
 #include "net.h"
 #include "sysemu.h"
 #include "boards.h"
-#include "pc.h" /* for the FPGA UART that emulates a 16550 */
+#include "serial.h"
 #include "imx.h"
 
     /* Memory map for Kzm Emulation Baseboard:
diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
index d4a8672..a3cb3ab 100644
--- a/hw/mips_fulong2e.c
+++ b/hw/mips_fulong2e.c
@@ -20,6 +20,7 @@
 
 #include "hw.h"
 #include "pc.h"
+#include "serial.h"
 #include "fdc.h"
 #include "net.h"
 #include "boards.h"
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index db927f1..d35cd54 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -26,6 +26,7 @@
 #include "mips.h"
 #include "mips_cpudevs.h"
 #include "pc.h"
+#include "serial.h"
 #include "isa.h"
 #include "fdc.h"
 #include "sysemu.h"
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 632b466..8f73b1b 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -24,6 +24,7 @@
 
 #include "hw.h"
 #include "pc.h"
+#include "serial.h"
 #include "fdc.h"
 #include "net.h"
 #include "boards.h"
diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
index 830f635..0ee6756 100644
--- a/hw/mips_mipssim.c
+++ b/hw/mips_mipssim.c
@@ -27,7 +27,7 @@
 #include "hw.h"
 #include "mips.h"
 #include "mips_cpudevs.h"
-#include "pc.h"
+#include "serial.h"
 #include "isa.h"
 #include "net.h"
 #include "sysemu.h"
diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
index 967a76e..b3be80b 100644
--- a/hw/mips_r4k.c
+++ b/hw/mips_r4k.c
@@ -11,6 +11,7 @@
 #include "mips.h"
 #include "mips_cpudevs.h"
 #include "pc.h"
+#include "serial.h"
 #include "isa.h"
 #include "net.h"
 #include "sysemu.h"
diff --git a/hw/musicpal.c b/hw/musicpal.c
index f305e21..346fe41 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -15,7 +15,7 @@
 #include "net.h"
 #include "sysemu.h"
 #include "boards.h"
-#include "pc.h"
+#include "serial.h"
 #include "qemu-timer.h"
 #include "ptimer.h"
 #include "block.h"
diff --git a/hw/omap_uart.c b/hw/omap_uart.c
index 167d5c4..1c16a54 100644
--- a/hw/omap_uart.c
+++ b/hw/omap_uart.c
@@ -20,8 +20,7 @@
 #include "qemu-char.h"
 #include "hw.h"
 #include "omap.h"
-/* We use pc-style serial ports.  */
-#include "pc.h"
+#include "serial.h"
 #include "exec-memory.h"
 
 /* UARTs */
diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c
index 55e97f0..e484613 100644
--- a/hw/openrisc_sim.c
+++ b/hw/openrisc_sim.c
@@ -21,7 +21,8 @@
 #include "hw.h"
 #include "boards.h"
 #include "elf.h"
-#include "pc.h"
+#include "serial.h"
+#include "net.h"
 #include "loader.h"
 #include "exec-memory.h"
 #include "sysemu.h"
diff --git a/hw/pc.c b/hw/pc.c
index 6c0722d..805e8ca 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -23,6 +23,7 @@
  */
 #include "hw.h"
 #include "pc.h"
+#include "serial.h"
 #include "apic.h"
 #include "fdc.h"
 #include "ide.h"
diff --git a/hw/pc.h b/hw/pc.h
index 9923d96..6cba7ce 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -12,33 +12,6 @@
 
 /* PC-style peripherals (also used by other machines).  */
 
-/* serial.c */
-
-SerialState *serial_init(int base, qemu_irq irq, int baudbase,
-                         CharDriverState *chr);
-SerialState *serial_mm_init(MemoryRegion *address_space,
-                            target_phys_addr_t base, int it_shift,
-                            qemu_irq irq, int baudbase,
-                            CharDriverState *chr, enum device_endian);
-static inline bool serial_isa_init(ISABus *bus, int index,
-                                   CharDriverState *chr)
-{
-    ISADevice *dev;
-
-    dev = isa_try_create(bus, "isa-serial");
-    if (!dev) {
-        return false;
-    }
-    qdev_prop_set_uint32(&dev->qdev, "index", index);
-    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
-    if (qdev_init(&dev->qdev) < 0) {
-        return false;
-    }
-    return true;
-}
-
-void serial_set_frequency(SerialState *s, uint32_t frequency);
-
 /* parallel.c */
 static inline bool parallel_init(ISABus *bus, int index, CharDriverState *chr)
 {
diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
index b9bfbed..1c12c5b 100644
--- a/hw/petalogix_ml605_mmu.c
+++ b/hw/petalogix_ml605_mmu.c
@@ -34,7 +34,7 @@
 #include "boards.h"
 #include "xilinx.h"
 #include "blockdev.h"
-#include "pc.h"
+#include "serial.h"
 #include "exec-memory.h"
 #include "ssi.h"
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index d23f9b2..846f53a 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -19,7 +19,7 @@
 #include "e500.h"
 #include "net.h"
 #include "hw/hw.h"
-#include "hw/pc.h"
+#include "hw/serial.h"
 #include "hw/pci.h"
 #include "hw/boards.h"
 #include "sysemu.h"
diff --git a/hw/ppc405_uc.c b/hw/ppc405_uc.c
index b52ab2f..e81409d 100644
--- a/hw/ppc405_uc.c
+++ b/hw/ppc405_uc.c
@@ -24,7 +24,7 @@
 #include "hw.h"
 #include "ppc.h"
 #include "ppc405.h"
-#include "pc.h"
+#include "serial.h"
 #include "qemu-timer.h"
 #include "sysemu.h"
 #include "qemu-log.h"
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index c198071..7e6fa85 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -23,7 +23,7 @@
 #include "loader.h"
 #include "elf.h"
 #include "exec-memory.h"
-#include "pc.h"
+#include "serial.h"
 #include "ppc.h"
 #include "ppc405.h"
 #include "sysemu.h"
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 1544430..50fd567 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -24,6 +24,7 @@
 #include "hw.h"
 #include "nvram.h"
 #include "pc.h"
+#include "serial.h"
 #include "fdc.h"
 #include "net.h"
 #include "sysemu.h"
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index d5f1420..4ec904f 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -10,7 +10,7 @@
 #include "sysbus.h"
 #include "pxa.h"
 #include "sysemu.h"
-#include "pc.h"
+#include "serial.h"
 #include "i2c.h"
 #include "ssi.h"
 #include "qemu-char.h"
diff --git a/hw/serial-isa.c b/hw/serial-isa.c
new file mode 100644
index 0000000..96c78f7
--- /dev/null
+++ b/hw/serial-isa.c
@@ -0,0 +1,130 @@
+/*
+ * QEMU 16550A UART emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "serial.h"
+#include "isa.h"
+
+typedef struct ISASerialState {
+    ISADevice dev;
+    uint32_t index;
+    uint32_t iobase;
+    uint32_t isairq;
+    SerialState state;
+} ISASerialState;
+
+static const int isa_serial_io[MAX_SERIAL_PORTS] = {
+    0x3f8, 0x2f8, 0x3e8, 0x2e8
+};
+static const int isa_serial_irq[MAX_SERIAL_PORTS] = {
+    4, 3, 4, 3
+};
+
+static int serial_isa_initfn(ISADevice *dev)
+{
+    static int index;
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+    SerialState *s = &isa->state;
+
+    if (isa->index == -1) {
+        isa->index = index;
+    }
+    if (isa->index >= MAX_SERIAL_PORTS) {
+        return -1;
+    }
+    if (isa->iobase == -1) {
+        isa->iobase = isa_serial_io[isa->index];
+    }
+    if (isa->isairq == -1) {
+        isa->isairq = isa_serial_irq[isa->index];
+    }
+    index++;
+
+    s->baudbase = 115200;
+    isa_init_irq(dev, &s->irq, isa->isairq);
+    serial_init_core(s);
+    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
+
+    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
+    isa_register_ioport(dev, &s->io, isa->iobase);
+    return 0;
+}
+
+static const VMStateDescription vmstate_isa_serial = {
+    .name = "serial",
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property serial_isa_properties[] = {
+    DEFINE_PROP_UINT32("index",  ISASerialState, index,   -1),
+    DEFINE_PROP_HEX32("iobase",  ISASerialState, iobase,  -1),
+    DEFINE_PROP_UINT32("irq",    ISASerialState, isairq,  -1),
+    DEFINE_PROP_CHR("chardev",   ISASerialState, state.chr),
+    DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void serial_isa_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+    ic->init = serial_isa_initfn;
+    dc->vmsd = &vmstate_isa_serial;
+    dc->props = serial_isa_properties;
+}
+
+static TypeInfo serial_isa_info = {
+    .name          = "isa-serial",
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(ISASerialState),
+    .class_init    = serial_isa_class_initfn,
+};
+
+static void serial_register_types(void)
+{
+    type_register_static(&serial_isa_info);
+}
+
+type_init(serial_register_types)
+
+bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
+{
+    ISADevice *dev;
+
+    dev = isa_try_create(bus, "isa-serial");
+    if (!dev) {
+        return false;
+    }
+    qdev_prop_set_uint32(&dev->qdev, "index", index);
+    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
+    if (qdev_init(&dev->qdev) < 0) {
+        return false;
+    }
+    return true;
+}
diff --git a/hw/serial.c b/hw/serial.c
index a421d1e..78e219d 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -22,12 +22,10 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-#include "hw.h"
+
+#include "serial.h"
 #include "qemu-char.h"
-#include "isa.h"
-#include "pc.h"
 #include "qemu-timer.h"
-#include "sysemu.h"
 
 //#define DEBUG_SERIAL
 
@@ -93,8 +91,6 @@
 #define UART_FCR_RFR        0x02    /* RCVR Fifo Reset */
 #define UART_FCR_FE         0x01    /* FIFO Enable */
 
-#define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
-
 #define XMIT_FIFO           0
 #define RECV_FIFO           1
 #define MAX_XMIT_RETRY      4
@@ -107,64 +103,6 @@ do { fprintf(stderr, "serial: " fmt , ## __VA_ARGS__); } while (0)
 do {} while (0)
 #endif
 
-typedef struct SerialFIFO {
-    uint8_t data[UART_FIFO_LENGTH];
-    uint8_t count;
-    uint8_t itl;                        /* Interrupt Trigger Level */
-    uint8_t tail;
-    uint8_t head;
-} SerialFIFO;
-
-struct SerialState {
-    uint16_t divider;
-    uint8_t rbr; /* receive register */
-    uint8_t thr; /* transmit holding register */
-    uint8_t tsr; /* transmit shift register */
-    uint8_t ier;
-    uint8_t iir; /* read only */
-    uint8_t lcr;
-    uint8_t mcr;
-    uint8_t lsr; /* read only */
-    uint8_t msr; /* read only */
-    uint8_t scr;
-    uint8_t fcr;
-    uint8_t fcr_vmstate; /* we can't write directly this value
-                            it has side effects */
-    /* NOTE: this hidden state is necessary for tx irq generation as
-       it can be reset while reading iir */
-    int thr_ipending;
-    qemu_irq irq;
-    CharDriverState *chr;
-    int last_break_enable;
-    int it_shift;
-    int baudbase;
-    int tsr_retry;
-    uint32_t wakeup;
-
-    uint64_t last_xmit_ts;              /* Time when the last byte was successfully sent out of the tsr */
-    SerialFIFO recv_fifo;
-    SerialFIFO xmit_fifo;
-
-    struct QEMUTimer *fifo_timeout_timer;
-    int timeout_ipending;                   /* timeout interrupt pending state */
-    struct QEMUTimer *transmit_timer;
-
-
-    uint64_t char_transmit_time;               /* time to transmit a char in ticks*/
-    int poll_msl;
-
-    struct QEMUTimer *modem_status_poll;
-    MemoryRegion io;
-};
-
-typedef struct ISASerialState {
-    ISADevice dev;
-    uint32_t index;
-    uint32_t iobase;
-    uint32_t isairq;
-    SerialState state;
-} ISASerialState;
-
 static void serial_receive1(void *opaque, const uint8_t *buf, int size);
 
 static void fifo_clear(SerialState *s, int fifo)
@@ -687,7 +625,7 @@ static int serial_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static const VMStateDescription vmstate_serial = {
+const VMStateDescription vmstate_serial = {
     .name = "serial",
     .version_id = 3,
     .minimum_version_id = 2,
@@ -736,7 +674,7 @@ static void serial_reset(void *opaque)
     qemu_irq_lower(s->irq);
 }
 
-static void serial_init_core(SerialState *s)
+void serial_init_core(SerialState *s)
 {
     if (!s->chr) {
         fprintf(stderr, "Can't create serial device, empty char device\n");
@@ -761,54 +699,15 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
     serial_update_parameters(s);
 }
 
-static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 static const MemoryRegionPortio serial_portio[] = {
     { 0, 8, 1, .read = serial_ioport_read, .write = serial_ioport_write },
     PORTIO_END_OF_LIST()
 };
 
-static const MemoryRegionOps serial_io_ops = {
+const MemoryRegionOps serial_io_ops = {
     .old_portio = serial_portio
 };
 
-static int serial_isa_initfn(ISADevice *dev)
-{
-    static int index;
-    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
-    SerialState *s = &isa->state;
-
-    if (isa->index == -1)
-        isa->index = index;
-    if (isa->index >= MAX_SERIAL_PORTS)
-        return -1;
-    if (isa->iobase == -1)
-        isa->iobase = isa_serial_io[isa->index];
-    if (isa->isairq == -1)
-        isa->isairq = isa_serial_irq[isa->index];
-    index++;
-
-    s->baudbase = 115200;
-    isa_init_irq(dev, &s->irq, isa->isairq);
-    serial_init_core(s);
-    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
-
-    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
-    isa_register_ioport(dev, &s->io, isa->iobase);
-    return 0;
-}
-
-static const VMStateDescription vmstate_isa_serial = {
-    .name = "serial",
-    .version_id = 3,
-    .minimum_version_id = 2,
-    .fields      = (VMStateField []) {
-        VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
                          CharDriverState *chr)
 {
@@ -886,35 +785,3 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
     serial_update_msl(s);
     return s;
 }
-
-static Property serial_isa_properties[] = {
-    DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
-    DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
-    DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
-    DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
-    DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void serial_isa_class_initfn(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
-    ic->init = serial_isa_initfn;
-    dc->vmsd = &vmstate_isa_serial;
-    dc->props = serial_isa_properties;
-}
-
-static TypeInfo serial_isa_info = {
-    .name          = "isa-serial",
-    .parent        = TYPE_ISA_DEVICE,
-    .instance_size = sizeof(ISASerialState),
-    .class_init    = serial_isa_class_initfn,
-};
-
-static void serial_register_types(void)
-{
-    type_register_static(&serial_isa_info);
-}
-
-type_init(serial_register_types)
diff --git a/hw/serial.h b/hw/serial.h
new file mode 100644
index 0000000..004a050
--- /dev/null
+++ b/hw/serial.h
@@ -0,0 +1,73 @@
+#include "hw.h"
+#include "sysemu.h"
+#include "memory.h"
+
+#define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
+
+typedef struct SerialFIFO {
+    uint8_t data[UART_FIFO_LENGTH];
+    uint8_t count;
+    uint8_t itl;                        /* Interrupt Trigger Level */
+    uint8_t tail;
+    uint8_t head;
+} SerialFIFO;
+
+struct SerialState {
+    uint16_t divider;
+    uint8_t rbr; /* receive register */
+    uint8_t thr; /* transmit holding register */
+    uint8_t tsr; /* transmit shift register */
+    uint8_t ier;
+    uint8_t iir; /* read only */
+    uint8_t lcr;
+    uint8_t mcr;
+    uint8_t lsr; /* read only */
+    uint8_t msr; /* read only */
+    uint8_t scr;
+    uint8_t fcr;
+    uint8_t fcr_vmstate; /* we can't write directly this value
+                            it has side effects */
+    /* NOTE: this hidden state is necessary for tx irq generation as
+       it can be reset while reading iir */
+    int thr_ipending;
+    qemu_irq irq;
+    CharDriverState *chr;
+    int last_break_enable;
+    int it_shift;
+    int baudbase;
+    int tsr_retry;
+    uint32_t wakeup;
+
+    /* Time when the last byte was successfully sent out of the tsr */
+    uint64_t last_xmit_ts;
+    SerialFIFO recv_fifo;
+    SerialFIFO xmit_fifo;
+
+    struct QEMUTimer *fifo_timeout_timer;
+    int timeout_ipending;           /* timeout interrupt pending state */
+    struct QEMUTimer *transmit_timer;
+
+
+    uint64_t char_transmit_time;    /* time to transmit a char in ticks */
+    int poll_msl;
+
+    struct QEMUTimer *modem_status_poll;
+    MemoryRegion io;
+};
+
+extern const VMStateDescription vmstate_serial;
+extern const MemoryRegionOps serial_io_ops;
+
+void serial_init_core(SerialState *s);
+void serial_set_frequency(SerialState *s, uint32_t frequency);
+
+/* legacy pre qom */
+SerialState *serial_init(int base, qemu_irq irq, int baudbase,
+                         CharDriverState *chr);
+SerialState *serial_mm_init(MemoryRegion *address_space,
+                            target_phys_addr_t base, int it_shift,
+                            qemu_irq irq, int baudbase,
+                            CharDriverState *chr, enum device_endian end);
+
+/* serial-isa.c */
+bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr);
diff --git a/hw/sm501.c b/hw/sm501.c
index 786e076..050d096 100644
--- a/hw/sm501.c
+++ b/hw/sm501.c
@@ -24,7 +24,7 @@
 
 #include <stdio.h>
 #include "hw.h"
-#include "pc.h"
+#include "serial.h"
 #include "console.h"
 #include "devices.h"
 #include "sysbus.h"
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 940db33..2397906 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -25,6 +25,7 @@
 #include "pci.h"
 #include "apb_pci.h"
 #include "pc.h"
+#include "serial.h"
 #include "nvram.h"
 #include "fdc.h"
 #include "net.h"
diff --git a/hw/virtex_ml507.c b/hw/virtex_ml507.c
index 79bc0d1..6ae5f60 100644
--- a/hw/virtex_ml507.c
+++ b/hw/virtex_ml507.c
@@ -24,7 +24,7 @@
 
 #include "sysbus.h"
 #include "hw.h"
-#include "pc.h"
+#include "serial.h"
 #include "net.h"
 #include "flash.h"
 #include "sysemu.h"
diff --git a/hw/xtensa_lx60.c b/hw/xtensa_lx60.c
index 3653f65..99d8b4f 100644
--- a/hw/xtensa_lx60.c
+++ b/hw/xtensa_lx60.c
@@ -31,7 +31,8 @@
 #include "elf.h"
 #include "memory.h"
 #include "exec-memory.h"
-#include "pc.h"
+#include "serial.h"
+#include "net.h"
 #include "sysbus.h"
 #include "flash.h"
 #include "blockdev.h"
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 2/9] serial: add pci variant
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 1/9] serial: split serial.c Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 3/9] serial: add windows inf file for the pci card to docs Gerd Hoffmann
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

So we get a hot-pluggable 16550 uart.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 default-configs/pci.mak |    2 +
 hw/Makefile.objs        |    1 +
 hw/pci_ids.h            |    1 +
 hw/serial-pci.c         |  115 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/serial.c             |    6 +++
 hw/serial.h             |    1 +
 6 files changed, 126 insertions(+), 0 deletions(-)
 create mode 100644 hw/serial-pci.c

diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 69e18f1..ae9d1eb 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -19,3 +19,5 @@ CONFIG_IDE_PCI=y
 CONFIG_AHCI=y
 CONFIG_ESP=y
 CONFIG_ESP_PCI=y
+CONFIG_SERIAL=y
+CONFIG_SERIAL_PCI=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 16e7a1e..af4ab0c 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -21,6 +21,7 @@ common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
 common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
+common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 301bf1c..c017a79 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -37,6 +37,7 @@
 #define PCI_CLASS_BRIDGE_PCI             0x0604
 #define PCI_CLASS_BRIDGE_OTHER           0x0680
 
+#define PCI_CLASS_COMMUNICATION_SERIAL   0x0700
 #define PCI_CLASS_COMMUNICATION_OTHER    0x0780
 
 #define PCI_CLASS_PROCESSOR_CO           0x0b40
diff --git a/hw/serial-pci.c b/hw/serial-pci.c
new file mode 100644
index 0000000..17247a8
--- /dev/null
+++ b/hw/serial-pci.c
@@ -0,0 +1,115 @@
+/*
+ * QEMU 16550A UART emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/*
+ * pci-serial spec:
+ *    pci region 0 is a io bar, 8 bytes long, with the 16550 uart mapped to it.
+ *    interrupt is wired to pin A.
+ *
+ * [root@fedora ~]# lspci -vnse
+ * 00:0e.0 0700: 1b36:0002 (rev 01) (prog-if 00 [8250])
+ *         Subsystem: 1af4:1100
+ *         Physical Slot: 14
+ *         Flags: fast devsel, IRQ 11
+ *         I/O ports at c130 [size=8]
+ *         Kernel driver in use: serial
+ */
+
+#include "serial.h"
+#include "pci.h"
+
+typedef struct PCISerialState {
+    PCIDevice dev;
+    SerialState state;
+} PCISerialState;
+
+static int serial_pci_init(PCIDevice *dev)
+{
+    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+    SerialState *s = &pci->state;
+
+    s->baudbase = 115200;
+    serial_init_core(s);
+
+    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
+    s->irq = pci->dev.irq[0];
+
+    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
+    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
+    return 0;
+}
+
+static void serial_pci_exit(PCIDevice *dev)
+{
+    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+    SerialState *s = &pci->state;
+
+    serial_exit_core(s);
+    memory_region_destroy(&s->io);
+}
+
+static const VMStateDescription vmstate_pci_serial = {
+    .name = "pci-serial",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCISerialState),
+        VMSTATE_STRUCT(state, PCISerialState, 0, vmstate_serial, SerialState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->init = serial_pci_init;
+    pc->exit = serial_pci_exit;
+    pc->vendor_id = 0x1b36; /* Red Hat */
+    pc->device_id = 0x0002;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_serial;
+    dc->props = serial_pci_properties;
+}
+
+static TypeInfo serial_pci_info = {
+    .name          = "pci-serial",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCISerialState),
+    .class_init    = serial_pci_class_initfn,
+};
+
+static void serial_pci_register_types(void)
+{
+    type_register_static(&serial_pci_info);
+}
+
+type_init(serial_pci_register_types)
diff --git a/hw/serial.c b/hw/serial.c
index 78e219d..5adbfaf 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -692,6 +692,12 @@ void serial_init_core(SerialState *s)
                           serial_event, s);
 }
 
+void serial_exit_core(SerialState *s)
+{
+    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL);
+    qemu_unregister_reset(serial_reset, s);
+}
+
 /* Change the main reference oscillator frequency. */
 void serial_set_frequency(SerialState *s, uint32_t frequency)
 {
diff --git a/hw/serial.h b/hw/serial.h
index 004a050..8fbd992 100644
--- a/hw/serial.h
+++ b/hw/serial.h
@@ -59,6 +59,7 @@ extern const VMStateDescription vmstate_serial;
 extern const MemoryRegionOps serial_io_ops;
 
 void serial_init_core(SerialState *s);
+void serial_exit_core(SerialState *s);
 void serial_set_frequency(SerialState *s, uint32_t frequency);
 
 /* legacy pre qom */
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 3/9] serial: add windows inf file for the pci card to docs
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 1/9] serial: split serial.c Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 2/9] serial: add pci variant Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 4/9] serial: add 2x + 4x pci variant Gerd Hoffmann
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/qemupciserial.inf |  107 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 docs/qemupciserial.inf

diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf
new file mode 100644
index 0000000..c7cea99
--- /dev/null
+++ b/docs/qemupciserial.inf
@@ -0,0 +1,107 @@
+; qemupciserial.inf for QEMU, based on MSPORTS.INF
+
+; The driver itself is shipped with Windows (serial.sys).  This is
+; just a inf file to tell windows which pci id the serial pci card
+; emulated by qemu has, and to apply a name tag to it which windows
+; will show in the device manager.
+
+; Installing the driver: Go to device manager.  You should find a "pci
+; serial card" tagged with a yellow question mark.  Open properties.
+; Pick "update driver".  Then "select driver manually".  Pick "Ports
+; (Com+Lpt)" from the list.  Click "Have a disk".  Select this file.
+; Procedure may vary a bit depending on the windows version.
+
+[Version]
+Signature="$CHICAGO$"
+Class=Ports
+ClassGuid={4D36E978-E325-11CE-BFC1-08002BE10318}
+Provider=%QEMU%
+DriverVer=09/24/2012,1.3.0
+
+[SourceDisksNames]
+3426=windows cd
+
+[SourceDisksFiles]
+serial.sys 		= 3426
+serenum.sys 		= 3426
+
+[DestinationDirs]
+DefaultDestDir  = 11        ;LDID_SYS
+ComPort.NT.Copy = 12        ;DIRID_DRIVERS
+SerialEnumerator.NT.Copy=12 ;DIRID_DRIVERS
+
+; Drivers
+;----------------------------------------------------------
+[Manufacturer]
+%QEMU%=QEMU,NTx86
+
+[QEMU.NTx86]
+%QEMU-PCI_SERIAL.DeviceDesc% = ComPort, "PCI\VEN_1b36&DEV_0002&CC_0700"
+
+; COM sections
+;----------------------------------------------------------
+[ComPort.AddReg]
+HKR,,PortSubClass,1,01
+
+[ComPort.NT]
+AddReg=ComPort.AddReg, ComPort.NT.AddReg
+LogConfig=caa
+SyssetupPnPFlags = 1
+
+[ComPort.NT.HW]
+AddReg=ComPort.NT.HW.AddReg
+
+[ComPort.NT.AddReg]
+HKR,,EnumPropPages32,,"MsPorts.dll,SerialPortPropPageProvider"
+
+[ComPort.NT.HW.AddReg]
+HKR,,"UpperFilters",0x00010000,"serenum"
+
+;-------------- Service installation
+; Port Driver (function driver for this device)
+[ComPort.NT.Services]
+AddService = Serial, 0x00000002, Serial_Service_Inst, Serial_EventLog_Inst
+AddService = Serenum,,Serenum_Service_Inst
+
+; -------------- Serial Port Driver install sections
+[Serial_Service_Inst]
+DisplayName    = %Serial.SVCDESC%
+ServiceType    = 1               ; SERVICE_KERNEL_DRIVER
+StartType      = 1               ; SERVICE_SYSTEM_START (this driver may do detection)
+ErrorControl   = 0               ; SERVICE_ERROR_IGNORE
+ServiceBinary  = %12%\serial.sys
+LoadOrderGroup = Extended base
+
+; -------------- Serenum Driver install section
+[Serenum_Service_Inst]
+DisplayName    = %Serenum.SVCDESC%
+ServiceType    = 1               ; SERVICE_KERNEL_DRIVER
+StartType      = 3               ; SERVICE_DEMAND_START
+ErrorControl   = 1               ; SERVICE_ERROR_NORMAL
+ServiceBinary  = %12%\serenum.sys
+LoadOrderGroup = PNP Filter
+
+[Serial_EventLog_Inst]
+AddReg = Serial_EventLog_AddReg
+
+[Serial_EventLog_AddReg]
+HKR,,EventMessageFile,0x00020000,"%%SystemRoot%%\System32\IoLogMsg.dll;%%SystemRoot%%\System32\drivers\serial.sys"
+HKR,,TypesSupported,0x00010001,7
+
+; The following sections are COM port resource configs.
+; Section name format means:
+; Char 1 = c (COM port)
+; Char 2 = I/O config: 1 (3f8), 2 (2f8), 3 (3e8), 4 (2e8), a (any)
+; Char 3 = IRQ config: #, a (any)
+
+[caa]                   ; Any base, any IRQ
+ConfigPriority=HARDRECONFIG
+IOConfig=8@100-ffff%fff8(3ff::)
+IRQConfig=S:3,4,5,7,9,10,11,12,14,15
+
+[Strings]
+QEMU="QEMU"
+QEMU-PCI_SERIAL.DeviceDesc="QEMU Serial PCI Card"
+
+Serial.SVCDESC   = "Serial port driver"
+Serenum.SVCDESC = "Serenum Filter Driver"
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 4/9] serial: add 2x + 4x pci variant
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 3/9] serial: add windows inf file for the pci card to docs Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15 14:18   ` Anthony Liguori
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 5/9] usb-serial: don't magically zap chardev on umplug Gerd Hoffmann
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add multiport serial card implementation, with two variants,
one featuring two and one featuring four ports.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/qemupciserial.inf |    2 +
 hw/serial-pci.c        |  157 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+), 0 deletions(-)

diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf
index c7cea99..3474310 100644
--- a/docs/qemupciserial.inf
+++ b/docs/qemupciserial.inf
@@ -11,6 +11,8 @@
 ; (Com+Lpt)" from the list.  Click "Have a disk".  Select this file.
 ; Procedure may vary a bit depending on the windows version.
 
+; FIXME: This file covers the single port version only.
+
 [Version]
 Signature="$CHICAGO$"
 Class=Ports
diff --git a/hw/serial-pci.c b/hw/serial-pci.c
index 17247a8..c89e8b0 100644
--- a/hw/serial-pci.c
+++ b/hw/serial-pci.c
@@ -28,6 +28,14 @@
  *    pci region 0 is a io bar, 8 bytes long, with the 16550 uart mapped to it.
  *    interrupt is wired to pin A.
  *
+ * pci-serial-4x spec:
+ *    pci region 0 is a io bar, with four 16550 uarts mapped after each other,
+ *    the first at offset 0, second at 8, third at 16 and fourth at 24.
+ *    interrupt is wired to pin A.
+ *
+ * pci-serial-2x spec:
+ *    same as pci-serial-4x but with two uarts only.
+ *
  * [root@fedora ~]# lspci -vnse
  * 00:0e.0 0700: 1b36:0002 (rev 01) (prog-if 00 [8250])
  *         Subsystem: 1af4:1100
@@ -40,11 +48,23 @@
 #include "serial.h"
 #include "pci.h"
 
+#define PCI_SERIAL_MAX_PORTS 4
+
 typedef struct PCISerialState {
     PCIDevice dev;
     SerialState state;
 } PCISerialState;
 
+typedef struct PCIMultiSerialState {
+    PCIDevice    dev;
+    MemoryRegion iobar;
+    uint32_t     ports;
+    char         *name[PCI_SERIAL_MAX_PORTS];
+    SerialState  state[PCI_SERIAL_MAX_PORTS];
+    uint32_t     level[PCI_SERIAL_MAX_PORTS];
+    qemu_irq     *irqs;
+} PCIMultiSerialState;
+
 static int serial_pci_init(PCIDevice *dev)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -61,6 +81,56 @@ static int serial_pci_init(PCIDevice *dev)
     return 0;
 }
 
+static void multi_serial_irq_mux(void *opaque, int n, int level)
+{
+    PCIMultiSerialState *pci = opaque;
+    int i, pending = 0;
+
+    pci->level[n] = level;
+    for (i = 0; i < pci->ports; i++) {
+        if (pci->level[i]) {
+            pending = 1;
+        }
+    }
+    qemu_set_irq(pci->dev.irq[0], pending);
+}
+
+static int multi_serial_pci_init(PCIDevice *dev)
+{
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    SerialState *s;
+    int i;
+
+    switch (pc->device_id) {
+    case 0x0003:
+        pci->ports = 2;
+        break;
+    case 0x0004:
+        pci->ports = 4;
+        break;
+    }
+    assert(pci->ports > 0);
+    assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
+
+    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
+    memory_region_init(&pci->iobar, "multiserial", 8 * pci->ports);
+    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
+    pci->irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci,
+                                   pci->ports);
+
+    for (i = 0; i < pci->ports; i++) {
+        s = pci->state + i;
+        s->baudbase = 115200;
+        serial_init_core(s);
+        s->irq = pci->irqs[i];
+        pci->name[i] = g_strdup_printf("uart #%d", i+1);
+        memory_region_init_io(&s->io, &serial_io_ops, s, pci->name[i], 8);
+        memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
+    }
+    return 0;
+}
+
 static void serial_pci_exit(PCIDevice *dev)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -70,6 +140,22 @@ static void serial_pci_exit(PCIDevice *dev)
     memory_region_destroy(&s->io);
 }
 
+static void multi_serial_pci_exit(PCIDevice *dev)
+{
+    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    SerialState *s;
+    int i;
+
+    for (i = 0; i < pci->ports; i++) {
+        s = pci->state + i;
+        serial_exit_core(s);
+        memory_region_destroy(&s->io);
+        g_free(pci->name[i]);
+    }
+    memory_region_destroy(&pci->iobar);
+    qemu_free_irqs(pci->irqs);
+}
+
 static const VMStateDescription vmstate_pci_serial = {
     .name = "pci-serial",
     .version_id = 1,
@@ -81,11 +167,38 @@ static const VMStateDescription vmstate_pci_serial = {
     }
 };
 
+static const VMStateDescription vmstate_pci_multi_serial = {
+    .name = "pci-serial-multi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState),
+        VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS,
+                             0, vmstate_serial, SerialState),
+        VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static Property serial_pci_properties[] = {
     DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static Property multi_2x_serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
+    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static Property multi_4x_serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
+    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+    DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
+    DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -100,6 +213,34 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
     dc->props = serial_pci_properties;
 }
 
+static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->init = multi_serial_pci_init;
+    pc->exit = multi_serial_pci_exit;
+    pc->vendor_id = 0x1b36; /* Red Hat */
+    pc->device_id = 0x0003;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_multi_serial;
+    dc->props = multi_2x_serial_pci_properties;
+}
+
+static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->init = multi_serial_pci_init;
+    pc->exit = multi_serial_pci_exit;
+    pc->vendor_id = 0x1b36; /* Red Hat */
+    pc->device_id = 0x0004;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_multi_serial;
+    dc->props = multi_4x_serial_pci_properties;
+}
+
 static TypeInfo serial_pci_info = {
     .name          = "pci-serial",
     .parent        = TYPE_PCI_DEVICE,
@@ -107,9 +248,25 @@ static TypeInfo serial_pci_info = {
     .class_init    = serial_pci_class_initfn,
 };
 
+static TypeInfo multi_2x_serial_pci_info = {
+    .name          = "pci-serial-2x",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIMultiSerialState),
+    .class_init    = multi_2x_serial_pci_class_initfn,
+};
+
+static TypeInfo multi_4x_serial_pci_info = {
+    .name          = "pci-serial-4x",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIMultiSerialState),
+    .class_init    = multi_4x_serial_pci_class_initfn,
+};
+
 static void serial_pci_register_types(void)
 {
     type_register_static(&serial_pci_info);
+    type_register_static(&multi_2x_serial_pci_info);
+    type_register_static(&multi_4x_serial_pci_info);
 }
 
 type_init(serial_pci_register_types)
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 5/9] usb-serial: don't magically zap chardev on umplug
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 4/9] serial: add 2x + 4x pci variant Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 6/9] usb-serial: only expose device in guest when the chardev is open Gerd Hoffmann
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 69b6e48..43214cd 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -421,7 +421,7 @@ static void usb_serial_handle_destroy(USBDevice *dev)
 {
     USBSerialState *s = (USBSerialState *)dev;
 
-    qemu_chr_delete(s->cs);
+    qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL);
 }
 
 static int usb_serial_can_read(void *opaque)
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 6/9] usb-serial: only expose device in guest when the chardev is open
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 5/9] usb-serial: don't magically zap chardev on umplug Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 43214cd..a466f99 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -427,6 +427,10 @@ static void usb_serial_handle_destroy(USBDevice *dev)
 static int usb_serial_can_read(void *opaque)
 {
     USBSerialState *s = opaque;
+
+    if (!s->dev.attached) {
+        return 0;
+    }
     return RECV_BUF - s->recv_used;
 }
 
@@ -469,8 +473,14 @@ static void usb_serial_event(void *opaque, int event)
         case CHR_EVENT_FOCUS:
             break;
         case CHR_EVENT_OPENED:
-            usb_serial_reset(s);
-            /* TODO: Reset USB port */
+            if (!s->dev.attached) {
+                usb_device_attach(&s->dev);
+            }
+            break;
+        case CHR_EVENT_CLOSED:
+            if (s->dev.attached) {
+                usb_device_detach(&s->dev);
+            }
             break;
     }
 }
@@ -481,6 +491,7 @@ static int usb_serial_initfn(USBDevice *dev)
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
+    dev->auto_attach = 0;
 
     if (!s->cs) {
         error_report("Property chardev is required");
@@ -490,6 +501,10 @@ static int usb_serial_initfn(USBDevice *dev)
     qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
                           usb_serial_event, s);
     usb_serial_handle_reset(dev);
+
+    if (s->cs->opened && !dev->attached) {
+        usb_device_attach(dev);
+    }
     return 0;
 }
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 6/9] usb-serial: only expose device in guest when the chardev is open Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-17  1:12   ` Luiz Capitulino
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 8/9] chardev: fix QemuOpts lifecycle Gerd Hoffmann
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |   24 +++++++++++++++---------
 qemu-char.h |    3 ++-
 vl.c        |    7 ++++++-
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index b082bae..e2e1da8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2755,19 +2755,20 @@ static const struct {
 };
 
 CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-                                    void (*init)(struct CharDriverState *s))
+                                    void (*init)(struct CharDriverState *s),
+                                    Error **errp)
 {
     CharDriverState *chr;
     int i;
 
     if (qemu_opts_id(opts) == NULL) {
-        fprintf(stderr, "chardev: no id specified\n");
+        error_setg(errp, "chardev: no id specified\n");
         return NULL;
     }
 
     if (qemu_opt_get(opts, "backend") == NULL) {
-        fprintf(stderr, "chardev: \"%s\" missing backend\n",
-                qemu_opts_id(opts));
+        error_setg(errp, "chardev: \"%s\" missing backend\n",
+                   qemu_opts_id(opts));
         return NULL;
     }
     for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
@@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
             break;
     }
     if (i == ARRAY_SIZE(backend_table)) {
-        fprintf(stderr, "chardev: backend \"%s\" not found\n",
-                qemu_opt_get(opts, "backend"));
+        error_setg(errp, "chardev: backend \"%s\" not found\n",
+                   qemu_opt_get(opts, "backend"));
         return NULL;
     }
 
     chr = backend_table[i].open(opts);
     if (!chr) {
-        fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
-                qemu_opt_get(opts, "backend"));
+        error_setg(errp, "chardev: opening backend \"%s\" failed\n",
+                   qemu_opt_get(opts, "backend"));
         return NULL;
     }
 
@@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     const char *p;
     CharDriverState *chr;
     QemuOpts *opts;
+    Error *err = NULL;
 
     if (strstart(filename, "chardev:", &p)) {
         return qemu_chr_find(p);
@@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     if (!opts)
         return NULL;
 
-    chr = qemu_chr_new_from_opts(opts, init);
+    chr = qemu_chr_new_from_opts(opts, init, &err);
+    if (error_is_set(&err)) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
+    }
     if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
         monitor_init(chr, MONITOR_USE_READLINE);
     }
diff --git a/qemu-char.h b/qemu-char.h
index 486644b..adae13e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -88,7 +88,8 @@ struct CharDriverState {
  * Returns: a new character backend
  */
 CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-                                    void (*init)(struct CharDriverState *s));
+                                    void (*init)(struct CharDriverState *s),
+                                    Error **errp);
 
 /**
  * @qemu_chr_new:
diff --git a/vl.c b/vl.c
index 5b357a3..35eb627 100644
--- a/vl.c
+++ b/vl.c
@@ -1940,8 +1940,13 @@ static int device_init_func(QemuOpts *opts, void *opaque)
 static int chardev_init_func(QemuOpts *opts, void *opaque)
 {
     CharDriverState *chr;
+    Error *err = NULL;
 
-    chr = qemu_chr_new_from_opts(opts, NULL);
+    chr = qemu_chr_new_from_opts(opts, NULL, &err);
+    if (error_is_set(&err)) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
+    }
     if (!chr)
         return -1;
     return 0;
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 8/9] chardev: fix QemuOpts lifecycle
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-17  1:18   ` Luiz Capitulino
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
  2012-10-17  1:52 ` [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Luiz Capitulino
  9 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

qemu_chr_new_from_opts handles QemuOpts release now, so callers don't
have to worry.  It will either be saved in CharDriverState, then
released in qemu_chr_delete, or in the error case released instantly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |   15 ++++++++++-----
 qemu-char.h |    1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index e2e1da8..be4ec61 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2763,13 +2763,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
 
     if (qemu_opts_id(opts) == NULL) {
         error_setg(errp, "chardev: no id specified\n");
-        return NULL;
+        goto err;
     }
 
     if (qemu_opt_get(opts, "backend") == NULL) {
         error_setg(errp, "chardev: \"%s\" missing backend\n",
                    qemu_opts_id(opts));
-        return NULL;
+        goto err;
     }
     for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
         if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0)
@@ -2778,14 +2778,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
     if (i == ARRAY_SIZE(backend_table)) {
         error_setg(errp, "chardev: backend \"%s\" not found\n",
                    qemu_opt_get(opts, "backend"));
-        return NULL;
+        goto err;
     }
 
     chr = backend_table[i].open(opts);
     if (!chr) {
         error_setg(errp, "chardev: opening backend \"%s\" failed\n",
                    qemu_opt_get(opts, "backend"));
-        return NULL;
+        goto err;
     }
 
     if (!chr->filename)
@@ -2806,7 +2806,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         chr->avail_connections = 1;
     }
     chr->label = g_strdup(qemu_opts_id(opts));
+    chr->opts = opts;
     return chr;
+
+err:
+    qemu_opts_del(opts);
+    return NULL;
 }
 
 CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s))
@@ -2832,7 +2837,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
         monitor_init(chr, MONITOR_USE_READLINE);
     }
-    qemu_opts_del(opts);
     return chr;
 }
 
@@ -2864,6 +2868,7 @@ void qemu_chr_delete(CharDriverState *chr)
         chr->chr_close(chr);
     g_free(chr->filename);
     g_free(chr->label);
+    qemu_opts_del(chr->opts);
     g_free(chr);
 }
 
diff --git a/qemu-char.h b/qemu-char.h
index adae13e..99bc132 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -74,6 +74,7 @@ struct CharDriverState {
     char *filename;
     int opened;
     int avail_connections;
+    QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (7 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 8/9] chardev: fix QemuOpts lifecycle Gerd Hoffmann
@ 2012-10-15  8:06 ` Gerd Hoffmann
  2012-10-15 14:22   ` Anthony Liguori
                     ` (3 more replies)
  2012-10-17  1:52 ` [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Luiz Capitulino
  9 siblings, 4 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-15  8:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch adds chardev_add and chardev_del monitor commands.

They work simliar to the netdev_{add,del} commands.  The hmp version of
chardev_add accepts like the -chardev command line option does.  The qmp
version expects the arguments being passed as named parameters.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
 hmp.c            |   23 ++++++++++++++++++++
 hmp.h            |    2 +
 qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
 qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
 qemu-char.h      |    1 +
 qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 202 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..48504d1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev_add",
+        .args_type  = "args:s",
+        .params     = "args",
+        .help       = "add chardev",
+        .mhandler.cmd = hmp_chardev_add,
+    },
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "del chardev",
+        .mhandler.cmd = hmp_chardev_del,
+    },
+
+STEXI
+@item chardev_del id
+@findex chardev_del
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 70bdec2..96bb900 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+    const char *args = qdict_get_str(qdict, "args");
+    Error *err = NULL;
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
+    if (opts == NULL) {
+        error_setg(&err, "Parsing chardev args failed\n");
+    } else {
+        qemu_chr_new_from_opts(opts, NULL, &err);
+    }
+    hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_del(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    qmp_chardev_del(qdict_get_str(qdict, "id"),
+                    &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..080afaa 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..550e4c7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,42 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev_add:
+#
+# Add a chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: the chardev backend: "file", "socket", ...
+# @path: file / device / unix socket path
+# @name: spice channel name
+# @host: host name
+# @port: port number
+# @server: create socket in server mode
+# @wait: wait for connect
+# @ipv4: force ipv4-only
+# @ipv6: force ipv6-only
+# @telnet: telnet negotiation
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_add', 'data': {'id'      : 'str',
+                                     'backend' : 'str',
+                                     '*props'  : '**' },
+  'gen': 'no' }
+
+##
+# @chardev_del:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_del', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index be4ec61..dae3e3c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+    Error *err = NULL;
+    QemuOptsList *opts_list;
+    QemuOpts *opts;
+
+    opts_list = qemu_find_opts_err("chardev", &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+
+    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+
+    qemu_chr_new_from_opts(opts, NULL, &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+    return 0;
+
+exit_err:
+    qerror_report_err(err);
+    error_free(err);
+    return -1;
+}
+
+void qmp_chardev_del(const char *id, Error **errp)
+{
+    CharDriverState *chr;
+
+    chr = qemu_chr_find(id);
+    if (NULL == chr) {
+        error_setg(errp, "Chardev '%s' not found\n", id);
+        return;
+    }
+    if (chr->chr_can_read || chr->chr_read ||
+        chr->chr_event || chr->handler_opaque) {
+        error_setg(errp, "Chardev '%s' is busy\n", id);
+        return;
+    }
+    qemu_chr_delete(chr);
+}
diff --git a/qemu-char.h b/qemu-char.h
index 99bc132..407e0fe 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -238,6 +238,7 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
 CharDriverState *qemu_chr_find(const char *name);
 
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
 
 /* add an eventfd to the qemu devices that are polled */
 CharDriverState *qemu_chr_open_eventfd(int eventfd);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..ebc1cee 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2576,3 +2576,64 @@ EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_target,
     },
+
+    {
+        .name       = "chardev_add",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_chardev_add,
+    },
+
+SQMP
+chardev_add
+-----------
+
+Add a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "backend": the chardev backend: "file", "socket", ... (json-string)
+- "path": file / device / unix socket path (json-string, optional)
+- "name": spice channel name (json-string, optional)
+- "host": host name (json-string, optional)
+- "port": port number (json-string, optional)
+- "server": create socket in server mode (json-bool, optional)
+- "wait": wait for connect (json-bool, optional)
+- "ipv4": force ipv4-only (json-bool, optional)
+- "ipv6": force ipv6-only (json-bool, optional)
+- "telnet": telnet negotiation (json-bool, optional)
+
+Example:
+
+-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
+                                              "backend" : "socket",
+                                              "path"    : "/tmp/foo",
+                                              "server"  : "on",
+                                              "wait"    : "off" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
+    },
+
+
+SQMP
+chardev_del
+-----------
+
+Remove a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must exist and not be in use (json-string)
+
+Example:
+
+-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
+<- { "return": {} }
+
+EQMP
-- 
1.7.1

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

* Re: [Qemu-devel] [PATCH v3 1/9] serial: split serial.c
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 1/9] serial: split serial.c Gerd Hoffmann
@ 2012-10-15 14:16   ` Anthony Liguori
  0 siblings, 0 replies; 22+ messages in thread
From: Anthony Liguori @ 2012-10-15 14:16 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

> Split serial.c into serial.c, serial.h and serial-isa.c.  While being at
> creating a serial.h header file move the serial prototypes from pc.h to
> the new serial.h.  The latter leads to s/pc.h/serial.h/ in tons of
> boards which just want the serial bits from pc.h
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/Makefile.objs         |    2 +-
>  hw/alpha_dp264.c         |    1 +
>  hw/kzm.c                 |    2 +-
>  hw/mips_fulong2e.c       |    1 +
>  hw/mips_jazz.c           |    1 +
>  hw/mips_malta.c          |    1 +
>  hw/mips_mipssim.c        |    2 +-
>  hw/mips_r4k.c            |    1 +
>  hw/musicpal.c            |    2 +-
>  hw/omap_uart.c           |    3 +-
>  hw/openrisc_sim.c        |    3 +-
>  hw/pc.c                  |    1 +
>  hw/pc.h                  |   27 ---------
>  hw/petalogix_ml605_mmu.c |    2 +-
>  hw/ppc/e500.c            |    2 +-
>  hw/ppc405_uc.c           |    2 +-
>  hw/ppc440_bamboo.c       |    2 +-
>  hw/ppc_prep.c            |    1 +
>  hw/pxa2xx.c              |    2 +-
>  hw/serial-isa.c          |  130 +++++++++++++++++++++++++++++++++++++++++
>  hw/serial.c              |  143 ++--------------------------------------------
>  hw/serial.h              |   73 +++++++++++++++++++++++
>  hw/sm501.c               |    2 +-
>  hw/sun4u.c               |    1 +
>  hw/virtex_ml507.c        |    2 +-
>  hw/xtensa_lx60.c         |    3 +-
>  26 files changed, 232 insertions(+), 180 deletions(-)
>  create mode 100644 hw/serial-isa.c
>  create mode 100644 hw/serial.h
>
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 854faa9..16e7a1e 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -20,7 +20,7 @@ common-obj-$(CONFIG_M48T59) += m48t59.o
>  common-obj-$(CONFIG_ESCC) += escc.o
>  common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
>  
> -common-obj-$(CONFIG_SERIAL) += serial.o
> +common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
>  common-obj-$(CONFIG_PARALLEL) += parallel.o
>  common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
>  common-obj-$(CONFIG_PCSPK) += pcspk.o
> diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
> index 5ea04c7..8ce04e5 100644
> --- a/hw/alpha_dp264.c
> +++ b/hw/alpha_dp264.c
> @@ -15,6 +15,7 @@
>  #include "mc146818rtc.h"
>  #include "ide.h"
>  #include "i8254.h"
> +#include "serial.h"
>  
>  #define MAX_IDE_BUS 2
>  
> diff --git a/hw/kzm.c b/hw/kzm.c
> index 68cd1b4..1f3082b 100644
> --- a/hw/kzm.c
> +++ b/hw/kzm.c
> @@ -21,7 +21,7 @@
>  #include "net.h"
>  #include "sysemu.h"
>  #include "boards.h"
> -#include "pc.h" /* for the FPGA UART that emulates a 16550 */
> +#include "serial.h"
>  #include "imx.h"
>  
>      /* Memory map for Kzm Emulation Baseboard:
> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
> index d4a8672..a3cb3ab 100644
> --- a/hw/mips_fulong2e.c
> +++ b/hw/mips_fulong2e.c
> @@ -20,6 +20,7 @@
>  
>  #include "hw.h"
>  #include "pc.h"
> +#include "serial.h"
>  #include "fdc.h"
>  #include "net.h"
>  #include "boards.h"
> diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> index db927f1..d35cd54 100644
> --- a/hw/mips_jazz.c
> +++ b/hw/mips_jazz.c
> @@ -26,6 +26,7 @@
>  #include "mips.h"
>  #include "mips_cpudevs.h"
>  #include "pc.h"
> +#include "serial.h"
>  #include "isa.h"
>  #include "fdc.h"
>  #include "sysemu.h"
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 632b466..8f73b1b 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -24,6 +24,7 @@
>  
>  #include "hw.h"
>  #include "pc.h"
> +#include "serial.h"
>  #include "fdc.h"
>  #include "net.h"
>  #include "boards.h"
> diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
> index 830f635..0ee6756 100644
> --- a/hw/mips_mipssim.c
> +++ b/hw/mips_mipssim.c
> @@ -27,7 +27,7 @@
>  #include "hw.h"
>  #include "mips.h"
>  #include "mips_cpudevs.h"
> -#include "pc.h"
> +#include "serial.h"
>  #include "isa.h"
>  #include "net.h"
>  #include "sysemu.h"
> diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
> index 967a76e..b3be80b 100644
> --- a/hw/mips_r4k.c
> +++ b/hw/mips_r4k.c
> @@ -11,6 +11,7 @@
>  #include "mips.h"
>  #include "mips_cpudevs.h"
>  #include "pc.h"
> +#include "serial.h"
>  #include "isa.h"
>  #include "net.h"
>  #include "sysemu.h"
> diff --git a/hw/musicpal.c b/hw/musicpal.c
> index f305e21..346fe41 100644
> --- a/hw/musicpal.c
> +++ b/hw/musicpal.c
> @@ -15,7 +15,7 @@
>  #include "net.h"
>  #include "sysemu.h"
>  #include "boards.h"
> -#include "pc.h"
> +#include "serial.h"
>  #include "qemu-timer.h"
>  #include "ptimer.h"
>  #include "block.h"
> diff --git a/hw/omap_uart.c b/hw/omap_uart.c
> index 167d5c4..1c16a54 100644
> --- a/hw/omap_uart.c
> +++ b/hw/omap_uart.c
> @@ -20,8 +20,7 @@
>  #include "qemu-char.h"
>  #include "hw.h"
>  #include "omap.h"
> -/* We use pc-style serial ports.  */
> -#include "pc.h"
> +#include "serial.h"
>  #include "exec-memory.h"
>  
>  /* UARTs */
> diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c
> index 55e97f0..e484613 100644
> --- a/hw/openrisc_sim.c
> +++ b/hw/openrisc_sim.c
> @@ -21,7 +21,8 @@
>  #include "hw.h"
>  #include "boards.h"
>  #include "elf.h"
> -#include "pc.h"
> +#include "serial.h"
> +#include "net.h"
>  #include "loader.h"
>  #include "exec-memory.h"
>  #include "sysemu.h"
> diff --git a/hw/pc.c b/hw/pc.c
> index 6c0722d..805e8ca 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -23,6 +23,7 @@
>   */
>  #include "hw.h"
>  #include "pc.h"
> +#include "serial.h"
>  #include "apic.h"
>  #include "fdc.h"
>  #include "ide.h"
> diff --git a/hw/pc.h b/hw/pc.h
> index 9923d96..6cba7ce 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -12,33 +12,6 @@
>  
>  /* PC-style peripherals (also used by other machines).  */
>  
> -/* serial.c */
> -
> -SerialState *serial_init(int base, qemu_irq irq, int baudbase,
> -                         CharDriverState *chr);
> -SerialState *serial_mm_init(MemoryRegion *address_space,
> -                            target_phys_addr_t base, int it_shift,
> -                            qemu_irq irq, int baudbase,
> -                            CharDriverState *chr, enum device_endian);
> -static inline bool serial_isa_init(ISABus *bus, int index,
> -                                   CharDriverState *chr)
> -{
> -    ISADevice *dev;
> -
> -    dev = isa_try_create(bus, "isa-serial");
> -    if (!dev) {
> -        return false;
> -    }
> -    qdev_prop_set_uint32(&dev->qdev, "index", index);
> -    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
> -    if (qdev_init(&dev->qdev) < 0) {
> -        return false;
> -    }
> -    return true;
> -}
> -
> -void serial_set_frequency(SerialState *s, uint32_t frequency);
> -
>  /* parallel.c */
>  static inline bool parallel_init(ISABus *bus, int index, CharDriverState *chr)
>  {
> diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
> index b9bfbed..1c12c5b 100644
> --- a/hw/petalogix_ml605_mmu.c
> +++ b/hw/petalogix_ml605_mmu.c
> @@ -34,7 +34,7 @@
>  #include "boards.h"
>  #include "xilinx.h"
>  #include "blockdev.h"
> -#include "pc.h"
> +#include "serial.h"
>  #include "exec-memory.h"
>  #include "ssi.h"
>  
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index d23f9b2..846f53a 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -19,7 +19,7 @@
>  #include "e500.h"
>  #include "net.h"
>  #include "hw/hw.h"
> -#include "hw/pc.h"
> +#include "hw/serial.h"
>  #include "hw/pci.h"
>  #include "hw/boards.h"
>  #include "sysemu.h"
> diff --git a/hw/ppc405_uc.c b/hw/ppc405_uc.c
> index b52ab2f..e81409d 100644
> --- a/hw/ppc405_uc.c
> +++ b/hw/ppc405_uc.c
> @@ -24,7 +24,7 @@
>  #include "hw.h"
>  #include "ppc.h"
>  #include "ppc405.h"
> -#include "pc.h"
> +#include "serial.h"
>  #include "qemu-timer.h"
>  #include "sysemu.h"
>  #include "qemu-log.h"
> diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
> index c198071..7e6fa85 100644
> --- a/hw/ppc440_bamboo.c
> +++ b/hw/ppc440_bamboo.c
> @@ -23,7 +23,7 @@
>  #include "loader.h"
>  #include "elf.h"
>  #include "exec-memory.h"
> -#include "pc.h"
> +#include "serial.h"
>  #include "ppc.h"
>  #include "ppc405.h"
>  #include "sysemu.h"
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 1544430..50fd567 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -24,6 +24,7 @@
>  #include "hw.h"
>  #include "nvram.h"
>  #include "pc.h"
> +#include "serial.h"
>  #include "fdc.h"
>  #include "net.h"
>  #include "sysemu.h"
> diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
> index d5f1420..4ec904f 100644
> --- a/hw/pxa2xx.c
> +++ b/hw/pxa2xx.c
> @@ -10,7 +10,7 @@
>  #include "sysbus.h"
>  #include "pxa.h"
>  #include "sysemu.h"
> -#include "pc.h"
> +#include "serial.h"
>  #include "i2c.h"
>  #include "ssi.h"
>  #include "qemu-char.h"
> diff --git a/hw/serial-isa.c b/hw/serial-isa.c
> new file mode 100644
> index 0000000..96c78f7
> --- /dev/null
> +++ b/hw/serial-isa.c
> @@ -0,0 +1,130 @@
> +/*
> + * QEMU 16550A UART emulation
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + * Copyright (c) 2008 Citrix Systems, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "serial.h"
> +#include "isa.h"
> +
> +typedef struct ISASerialState {
> +    ISADevice dev;
> +    uint32_t index;
> +    uint32_t iobase;
> +    uint32_t isairq;
> +    SerialState state;
> +} ISASerialState;
> +
> +static const int isa_serial_io[MAX_SERIAL_PORTS] = {
> +    0x3f8, 0x2f8, 0x3e8, 0x2e8
> +};
> +static const int isa_serial_irq[MAX_SERIAL_PORTS] = {
> +    4, 3, 4, 3
> +};
> +
> +static int serial_isa_initfn(ISADevice *dev)
> +{
> +    static int index;
> +    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
> +    SerialState *s = &isa->state;
> +
> +    if (isa->index == -1) {
> +        isa->index = index;
> +    }
> +    if (isa->index >= MAX_SERIAL_PORTS) {
> +        return -1;
> +    }
> +    if (isa->iobase == -1) {
> +        isa->iobase = isa_serial_io[isa->index];
> +    }
> +    if (isa->isairq == -1) {
> +        isa->isairq = isa_serial_irq[isa->index];
> +    }
> +    index++;
> +
> +    s->baudbase = 115200;
> +    isa_init_irq(dev, &s->irq, isa->isairq);
> +    serial_init_core(s);
> +    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
> +
> +    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
> +    isa_register_ioport(dev, &s->io, isa->iobase);
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_isa_serial = {
> +    .name = "serial",
> +    .version_id = 3,
> +    .minimum_version_id = 2,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property serial_isa_properties[] = {
> +    DEFINE_PROP_UINT32("index",  ISASerialState, index,   -1),
> +    DEFINE_PROP_HEX32("iobase",  ISASerialState, iobase,  -1),
> +    DEFINE_PROP_UINT32("irq",    ISASerialState, isairq,  -1),
> +    DEFINE_PROP_CHR("chardev",   ISASerialState, state.chr),
> +    DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void serial_isa_class_initfn(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> +    ic->init = serial_isa_initfn;
> +    dc->vmsd = &vmstate_isa_serial;
> +    dc->props = serial_isa_properties;
> +}
> +
> +static TypeInfo serial_isa_info = {
> +    .name          = "isa-serial",
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(ISASerialState),
> +    .class_init    = serial_isa_class_initfn,
> +};
> +
> +static void serial_register_types(void)
> +{
> +    type_register_static(&serial_isa_info);
> +}
> +
> +type_init(serial_register_types)
> +
> +bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
> +{
> +    ISADevice *dev;
> +
> +    dev = isa_try_create(bus, "isa-serial");
> +    if (!dev) {
> +        return false;
> +    }
> +    qdev_prop_set_uint32(&dev->qdev, "index", index);
> +    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
> +    if (qdev_init(&dev->qdev) < 0) {
> +        return false;
> +    }
> +    return true;
> +}
> diff --git a/hw/serial.c b/hw/serial.c
> index a421d1e..78e219d 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -22,12 +22,10 @@
>   * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>   * THE SOFTWARE.
>   */
> -#include "hw.h"
> +
> +#include "serial.h"
>  #include "qemu-char.h"
> -#include "isa.h"
> -#include "pc.h"
>  #include "qemu-timer.h"
> -#include "sysemu.h"
>  
>  //#define DEBUG_SERIAL
>  
> @@ -93,8 +91,6 @@
>  #define UART_FCR_RFR        0x02    /* RCVR Fifo Reset */
>  #define UART_FCR_FE         0x01    /* FIFO Enable */
>  
> -#define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
> -
>  #define XMIT_FIFO           0
>  #define RECV_FIFO           1
>  #define MAX_XMIT_RETRY      4
> @@ -107,64 +103,6 @@ do { fprintf(stderr, "serial: " fmt , ## __VA_ARGS__); } while (0)
>  do {} while (0)
>  #endif
>  
> -typedef struct SerialFIFO {
> -    uint8_t data[UART_FIFO_LENGTH];
> -    uint8_t count;
> -    uint8_t itl;                        /* Interrupt Trigger Level */
> -    uint8_t tail;
> -    uint8_t head;
> -} SerialFIFO;
> -
> -struct SerialState {
> -    uint16_t divider;
> -    uint8_t rbr; /* receive register */
> -    uint8_t thr; /* transmit holding register */
> -    uint8_t tsr; /* transmit shift register */
> -    uint8_t ier;
> -    uint8_t iir; /* read only */
> -    uint8_t lcr;
> -    uint8_t mcr;
> -    uint8_t lsr; /* read only */
> -    uint8_t msr; /* read only */
> -    uint8_t scr;
> -    uint8_t fcr;
> -    uint8_t fcr_vmstate; /* we can't write directly this value
> -                            it has side effects */
> -    /* NOTE: this hidden state is necessary for tx irq generation as
> -       it can be reset while reading iir */
> -    int thr_ipending;
> -    qemu_irq irq;
> -    CharDriverState *chr;
> -    int last_break_enable;
> -    int it_shift;
> -    int baudbase;
> -    int tsr_retry;
> -    uint32_t wakeup;
> -
> -    uint64_t last_xmit_ts;              /* Time when the last byte was successfully sent out of the tsr */
> -    SerialFIFO recv_fifo;
> -    SerialFIFO xmit_fifo;
> -
> -    struct QEMUTimer *fifo_timeout_timer;
> -    int timeout_ipending;                   /* timeout interrupt pending state */
> -    struct QEMUTimer *transmit_timer;
> -
> -
> -    uint64_t char_transmit_time;               /* time to transmit a char in ticks*/
> -    int poll_msl;
> -
> -    struct QEMUTimer *modem_status_poll;
> -    MemoryRegion io;
> -};
> -
> -typedef struct ISASerialState {
> -    ISADevice dev;
> -    uint32_t index;
> -    uint32_t iobase;
> -    uint32_t isairq;
> -    SerialState state;
> -} ISASerialState;
> -
>  static void serial_receive1(void *opaque, const uint8_t *buf, int size);
>  
>  static void fifo_clear(SerialState *s, int fifo)
> @@ -687,7 +625,7 @@ static int serial_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> -static const VMStateDescription vmstate_serial = {
> +const VMStateDescription vmstate_serial = {
>      .name = "serial",
>      .version_id = 3,
>      .minimum_version_id = 2,
> @@ -736,7 +674,7 @@ static void serial_reset(void *opaque)
>      qemu_irq_lower(s->irq);
>  }
>  
> -static void serial_init_core(SerialState *s)
> +void serial_init_core(SerialState *s)
>  {
>      if (!s->chr) {
>          fprintf(stderr, "Can't create serial device, empty char device\n");
> @@ -761,54 +699,15 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
>      serial_update_parameters(s);
>  }
>  
> -static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
> -static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
> -
>  static const MemoryRegionPortio serial_portio[] = {
>      { 0, 8, 1, .read = serial_ioport_read, .write = serial_ioport_write },
>      PORTIO_END_OF_LIST()
>  };
>  
> -static const MemoryRegionOps serial_io_ops = {
> +const MemoryRegionOps serial_io_ops = {
>      .old_portio = serial_portio
>  };
>  
> -static int serial_isa_initfn(ISADevice *dev)
> -{
> -    static int index;
> -    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
> -    SerialState *s = &isa->state;
> -
> -    if (isa->index == -1)
> -        isa->index = index;
> -    if (isa->index >= MAX_SERIAL_PORTS)
> -        return -1;
> -    if (isa->iobase == -1)
> -        isa->iobase = isa_serial_io[isa->index];
> -    if (isa->isairq == -1)
> -        isa->isairq = isa_serial_irq[isa->index];
> -    index++;
> -
> -    s->baudbase = 115200;
> -    isa_init_irq(dev, &s->irq, isa->isairq);
> -    serial_init_core(s);
> -    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
> -
> -    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
> -    isa_register_ioport(dev, &s->io, isa->iobase);
> -    return 0;
> -}
> -
> -static const VMStateDescription vmstate_isa_serial = {
> -    .name = "serial",
> -    .version_id = 3,
> -    .minimum_version_id = 2,
> -    .fields      = (VMStateField []) {
> -        VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
>  SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>                           CharDriverState *chr)
>  {
> @@ -886,35 +785,3 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>      serial_update_msl(s);
>      return s;
>  }
> -
> -static Property serial_isa_properties[] = {
> -    DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
> -    DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
> -    DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
> -    DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
> -    DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void serial_isa_class_initfn(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> -    ic->init = serial_isa_initfn;
> -    dc->vmsd = &vmstate_isa_serial;
> -    dc->props = serial_isa_properties;
> -}
> -
> -static TypeInfo serial_isa_info = {
> -    .name          = "isa-serial",
> -    .parent        = TYPE_ISA_DEVICE,
> -    .instance_size = sizeof(ISASerialState),
> -    .class_init    = serial_isa_class_initfn,
> -};
> -
> -static void serial_register_types(void)
> -{
> -    type_register_static(&serial_isa_info);
> -}
> -
> -type_init(serial_register_types)
> diff --git a/hw/serial.h b/hw/serial.h
> new file mode 100644
> index 0000000..004a050
> --- /dev/null
> +++ b/hw/serial.h
> @@ -0,0 +1,73 @@

Please make sure to include a license in all new files.

> +#include "hw.h"
> +#include "sysemu.h"
> +#include "memory.h"
> +
> +#define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
> +
> +typedef struct SerialFIFO {
> +    uint8_t data[UART_FIFO_LENGTH];
> +    uint8_t count;
> +    uint8_t itl;                        /* Interrupt Trigger Level */
> +    uint8_t tail;
> +    uint8_t head;
> +} SerialFIFO;
> +
> +struct SerialState {
> +    uint16_t divider;
> +    uint8_t rbr; /* receive register */
> +    uint8_t thr; /* transmit holding register */
> +    uint8_t tsr; /* transmit shift register */
> +    uint8_t ier;
> +    uint8_t iir; /* read only */
> +    uint8_t lcr;
> +    uint8_t mcr;
> +    uint8_t lsr; /* read only */
> +    uint8_t msr; /* read only */
> +    uint8_t scr;
> +    uint8_t fcr;
> +    uint8_t fcr_vmstate; /* we can't write directly this value
> +                            it has side effects */
> +    /* NOTE: this hidden state is necessary for tx irq generation as
> +       it can be reset while reading iir */
> +    int thr_ipending;
> +    qemu_irq irq;
> +    CharDriverState *chr;
> +    int last_break_enable;
> +    int it_shift;
> +    int baudbase;
> +    int tsr_retry;
> +    uint32_t wakeup;
> +
> +    /* Time when the last byte was successfully sent out of the tsr */
> +    uint64_t last_xmit_ts;
> +    SerialFIFO recv_fifo;
> +    SerialFIFO xmit_fifo;
> +
> +    struct QEMUTimer *fifo_timeout_timer;
> +    int timeout_ipending;           /* timeout interrupt pending state */
> +    struct QEMUTimer *transmit_timer;
> +
> +
> +    uint64_t char_transmit_time;    /* time to transmit a char in ticks */
> +    int poll_msl;
> +
> +    struct QEMUTimer *modem_status_poll;
> +    MemoryRegion io;
> +};
> +
> +extern const VMStateDescription vmstate_serial;
> +extern const MemoryRegionOps serial_io_ops;
> +
> +void serial_init_core(SerialState *s);

Instead of exposing all of this, another option would be to QOM-ify
SerialState such that it can be created by doing object_init() as a
proper child device.

Not a pre-req for this series (it can be done as a follow up).

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH v3 4/9] serial: add 2x + 4x pci variant
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 4/9] serial: add 2x + 4x pci variant Gerd Hoffmann
@ 2012-10-15 14:18   ` Anthony Liguori
  0 siblings, 0 replies; 22+ messages in thread
From: Anthony Liguori @ 2012-10-15 14:18 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

> Add multiport serial card implementation, with two variants,
> one featuring two and one featuring four ports.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  docs/qemupciserial.inf |    2 +
>  hw/serial-pci.c        |  157 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 159 insertions(+), 0 deletions(-)
>
> diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf
> index c7cea99..3474310 100644
> --- a/docs/qemupciserial.inf
> +++ b/docs/qemupciserial.inf
> @@ -11,6 +11,8 @@
>  ; (Com+Lpt)" from the list.  Click "Have a disk".  Select this file.
>  ; Procedure may vary a bit depending on the windows version.
>  
> +; FIXME: This file covers the single port version only.
> +
>  [Version]
>  Signature="$CHICAGO$"
>  Class=Ports
> diff --git a/hw/serial-pci.c b/hw/serial-pci.c
> index 17247a8..c89e8b0 100644
> --- a/hw/serial-pci.c
> +++ b/hw/serial-pci.c
> @@ -28,6 +28,14 @@
>   *    pci region 0 is a io bar, 8 bytes long, with the 16550 uart mapped to it.
>   *    interrupt is wired to pin A.
>   *
> + * pci-serial-4x spec:
> + *    pci region 0 is a io bar, with four 16550 uarts mapped after each other,
> + *    the first at offset 0, second at 8, third at 16 and fourth at 24.
> + *    interrupt is wired to pin A.
> + *
> + * pci-serial-2x spec:
> + *    same as pci-serial-4x but with two uarts only.
> + *

I know I neglected to respond to your previous note in this thread, but
I'd prefer this be made a file in docs/.

That way, you can look in docs and discover all of the QEMU-invented
devices.  Having to troll through hw/*.c to find which new devices were
added is a bit painful.

This is important for people looking to implement guest support for
QEMU.

Regards,

Anthony Liguori

>   * [root@fedora ~]# lspci -vnse
>   * 00:0e.0 0700: 1b36:0002 (rev 01) (prog-if 00 [8250])
>   *         Subsystem: 1af4:1100
> @@ -40,11 +48,23 @@
>  #include "serial.h"
>  #include "pci.h"
>  
> +#define PCI_SERIAL_MAX_PORTS 4
> +
>  typedef struct PCISerialState {
>      PCIDevice dev;
>      SerialState state;
>  } PCISerialState;
>  
> +typedef struct PCIMultiSerialState {
> +    PCIDevice    dev;
> +    MemoryRegion iobar;
> +    uint32_t     ports;
> +    char         *name[PCI_SERIAL_MAX_PORTS];
> +    SerialState  state[PCI_SERIAL_MAX_PORTS];
> +    uint32_t     level[PCI_SERIAL_MAX_PORTS];
> +    qemu_irq     *irqs;
> +} PCIMultiSerialState;
> +
>  static int serial_pci_init(PCIDevice *dev)
>  {
>      PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
> @@ -61,6 +81,56 @@ static int serial_pci_init(PCIDevice *dev)
>      return 0;
>  }
>  
> +static void multi_serial_irq_mux(void *opaque, int n, int level)
> +{
> +    PCIMultiSerialState *pci = opaque;
> +    int i, pending = 0;
> +
> +    pci->level[n] = level;
> +    for (i = 0; i < pci->ports; i++) {
> +        if (pci->level[i]) {
> +            pending = 1;
> +        }
> +    }
> +    qemu_set_irq(pci->dev.irq[0], pending);
> +}
> +
> +static int multi_serial_pci_init(PCIDevice *dev)
> +{
> +    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
> +    SerialState *s;
> +    int i;
> +
> +    switch (pc->device_id) {
> +    case 0x0003:
> +        pci->ports = 2;
> +        break;
> +    case 0x0004:
> +        pci->ports = 4;
> +        break;
> +    }
> +    assert(pci->ports > 0);
> +    assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
> +
> +    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
> +    memory_region_init(&pci->iobar, "multiserial", 8 * pci->ports);
> +    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
> +    pci->irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci,
> +                                   pci->ports);
> +
> +    for (i = 0; i < pci->ports; i++) {
> +        s = pci->state + i;
> +        s->baudbase = 115200;
> +        serial_init_core(s);
> +        s->irq = pci->irqs[i];
> +        pci->name[i] = g_strdup_printf("uart #%d", i+1);
> +        memory_region_init_io(&s->io, &serial_io_ops, s, pci->name[i], 8);
> +        memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
> +    }
> +    return 0;
> +}
> +
>  static void serial_pci_exit(PCIDevice *dev)
>  {
>      PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
> @@ -70,6 +140,22 @@ static void serial_pci_exit(PCIDevice *dev)
>      memory_region_destroy(&s->io);
>  }
>  
> +static void multi_serial_pci_exit(PCIDevice *dev)
> +{
> +    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
> +    SerialState *s;
> +    int i;
> +
> +    for (i = 0; i < pci->ports; i++) {
> +        s = pci->state + i;
> +        serial_exit_core(s);
> +        memory_region_destroy(&s->io);
> +        g_free(pci->name[i]);
> +    }
> +    memory_region_destroy(&pci->iobar);
> +    qemu_free_irqs(pci->irqs);
> +}
> +
>  static const VMStateDescription vmstate_pci_serial = {
>      .name = "pci-serial",
>      .version_id = 1,
> @@ -81,11 +167,38 @@ static const VMStateDescription vmstate_pci_serial = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_pci_multi_serial = {
> +    .name = "pci-serial-multi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState),
> +        VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS,
> +                             0, vmstate_serial, SerialState),
> +        VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static Property serial_pci_properties[] = {
>      DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static Property multi_2x_serial_pci_properties[] = {
> +    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
> +    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static Property multi_4x_serial_pci_properties[] = {
> +    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
> +    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
> +    DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
> +    DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void serial_pci_class_initfn(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -100,6 +213,34 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
>      dc->props = serial_pci_properties;
>  }
>  
> +static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
> +    pc->init = multi_serial_pci_init;
> +    pc->exit = multi_serial_pci_exit;
> +    pc->vendor_id = 0x1b36; /* Red Hat */
> +    pc->device_id = 0x0003;
> +    pc->revision = 1;
> +    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
> +    dc->vmsd = &vmstate_pci_multi_serial;
> +    dc->props = multi_2x_serial_pci_properties;
> +}
> +
> +static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
> +    pc->init = multi_serial_pci_init;
> +    pc->exit = multi_serial_pci_exit;
> +    pc->vendor_id = 0x1b36; /* Red Hat */
> +    pc->device_id = 0x0004;
> +    pc->revision = 1;
> +    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
> +    dc->vmsd = &vmstate_pci_multi_serial;
> +    dc->props = multi_4x_serial_pci_properties;
> +}
> +
>  static TypeInfo serial_pci_info = {
>      .name          = "pci-serial",
>      .parent        = TYPE_PCI_DEVICE,
> @@ -107,9 +248,25 @@ static TypeInfo serial_pci_info = {
>      .class_init    = serial_pci_class_initfn,
>  };
>  
> +static TypeInfo multi_2x_serial_pci_info = {
> +    .name          = "pci-serial-2x",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCIMultiSerialState),
> +    .class_init    = multi_2x_serial_pci_class_initfn,
> +};
> +
> +static TypeInfo multi_4x_serial_pci_info = {
> +    .name          = "pci-serial-4x",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCIMultiSerialState),
> +    .class_init    = multi_4x_serial_pci_class_initfn,
> +};
> +
>  static void serial_pci_register_types(void)
>  {
>      type_register_static(&serial_pci_info);
> +    type_register_static(&multi_2x_serial_pci_info);
> +    type_register_static(&multi_4x_serial_pci_info);
>  }
>  
>  type_init(serial_pci_register_types)
> -- 
> 1.7.1

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

* Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
@ 2012-10-15 14:22   ` Anthony Liguori
  2012-10-15 18:04   ` Eric Blake
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Anthony Liguori @ 2012-10-15 14:22 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch adds chardev_add and chardev_del monitor commands.
>
> They work simliar to the netdev_{add,del} commands.  The hmp version of
> chardev_add accepts like the -chardev command line option does.  The qmp
> version expects the arguments being passed as named parameters.
>
> chardev_del just takes an id argument and zaps the chardev specified.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>  hmp.c            |   23 ++++++++++++++++++++
>  hmp.h            |    2 +
>  qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>  qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
>  qemu-char.h      |    1 +
>  qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 202 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
>  ETEXI
>  
>      {
> +        .name       = "chardev_add",
> +        .args_type  = "args:s",
> +        .params     = "args",
> +        .help       = "add chardev",
> +        .mhandler.cmd = hmp_chardev_add,
> +    },

Should be chardev-add at this point.

> +
> +STEXI
> +@item chardev_add args
> +@findex chardev_add
> +
> +chardev_add accepts the same parameters as the -chardev command line switch.
> +
> +ETEXI
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "id:s",
> +        .params     = "id",
> +        .help       = "del chardev",
> +        .mhandler.cmd = hmp_chardev_del,
> +    },
> +
> +STEXI
> +@item chardev_del id
> +@findex chardev_del
> +
> +Removes the chardev @var{id}.
> +
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 70bdec2..96bb900 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>      qmp_screendump(filename, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
> +{
> +    const char *args = qdict_get_str(qdict, "args");
> +    Error *err = NULL;
> +    QemuOpts *opts;
> +
> +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
> +    if (opts == NULL) {
> +        error_setg(&err, "Parsing chardev args failed\n");
> +    } else {
> +        qemu_chr_new_from_opts(opts, NULL, &err);
> +    }
> +    hmp_handle_error(mon, &err);
> +}
> +
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    qmp_chardev_del(qdict_get_str(qdict, "id"),
> +                    &err);
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 71ea384..080afaa 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>  void hmp_closefd(Monitor *mon, const QDict *qdict);
>  void hmp_send_key(Monitor *mon, const QDict *qdict);
>  void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f9dbdae..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'screendump', 'data': {'filename': 'str'} }
> +
> +##
> +# @chardev_add:
> +#
> +# Add a chardev
> +#
> +# @id: the chardev's ID, must be unique
> +# @backend: the chardev backend: "file", "socket", ...
> +# @path: file / device / unix socket path
> +# @name: spice channel name
> +# @host: host name
> +# @port: port number
> +# @server: create socket in server mode
> +# @wait: wait for connect
> +# @ipv4: force ipv4-only
> +# @ipv6: force ipv6-only
> +# @telnet: telnet negotiation

If we're documenting all of the options, then we probably should specify
them in the schema.  It's a little more cumbersome with QemuOpts, but it
makes the schema more correct wrt the implementation.

Regards,

Anthony Liguori

> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @chardev_del:
> +#
> +# Remove a chardev
> +#
> +# @id: the chardev's ID, must exist and not be in use
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_del', 'data': {'id': 'str'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index be4ec61..dae3e3c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
>      return serial_hds[next_serial++];
>  }
>  
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +void qmp_chardev_del(const char *id, Error **errp)
> +{
> +    CharDriverState *chr;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL == chr) {
> +        error_setg(errp, "Chardev '%s' not found\n", id);
> +        return;
> +    }
> +    if (chr->chr_can_read || chr->chr_read ||
> +        chr->chr_event || chr->handler_opaque) {
> +        error_setg(errp, "Chardev '%s' is busy\n", id);
> +        return;
> +    }
> +    qemu_chr_delete(chr);
> +}
> diff --git a/qemu-char.h b/qemu-char.h
> index 99bc132..407e0fe 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -238,6 +238,7 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
>  CharDriverState *qemu_chr_find(const char *name);
>  
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>  
>  /* add an eventfd to the qemu devices that are polled */
>  CharDriverState *qemu_chr_open_eventfd(int eventfd);
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..ebc1cee 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2576,3 +2576,64 @@ EQMP
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_input_query_target,
>      },
> +
> +    {
> +        .name       = "chardev_add",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_chardev_add,
> +    },
> +
> +SQMP
> +chardev_add
> +-----------
> +
> +Add a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must be unique (json-string)
> +- "backend": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)
> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +
> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",
> +                                              "wait"    : "off" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
> +    },
> +
> +
> +SQMP
> +chardev_del
> +-----------
> +
> +Remove a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must exist and not be in use (json-string)
> +
> +Example:
> +
> +-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
> +<- { "return": {} }
> +
> +EQMP
> -- 
> 1.7.1

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

* Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
  2012-10-15 14:22   ` Anthony Liguori
@ 2012-10-15 18:04   ` Eric Blake
  2012-10-17  1:49     ` Luiz Capitulino
  2012-10-16  9:13   ` Lei Li
  2012-10-17  1:28   ` Luiz Capitulino
  3 siblings, 1 reply; 22+ messages in thread
From: Eric Blake @ 2012-10-15 18:04 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2976 bytes --]

On 10/15/2012 02:06 AM, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_del monitor commands.
> 
> They work simliar to the netdev_{add,del} commands.  The hmp version of

s/simliar/similar/

> chardev_add accepts like the -chardev command line option does.  The qmp
> version expects the arguments being passed as named parameters.
> 
> chardev_del just takes an id argument and zaps the chardev specified.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'screendump', 'data': {'filename': 'str'} }
> +
> +##
> +# @chardev_add:

chardev-add

> +#
> +# Add a chardev
> +#
> +# @id: the chardev's ID, must be unique
> +# @backend: the chardev backend: "file", "socket", ...

Should this be an enum type, instead of an open-coded string?

> +# @path: file / device / unix socket path
> +# @name: spice channel name
> +# @host: host name
> +# @port: port number
> +# @server: create socket in server mode
> +# @wait: wait for connect
> +# @ipv4: force ipv4-only
> +# @ipv6: force ipv6-only
> +# @telnet: telnet negotiation
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     '*props'  : '**' },

Having an open-coded list for props feels awkward; it would be nicer to
have the schema completely describe everything, even though that may be
more documentation work.

> +  'gen': 'no' }
> +
> +##
> +# @chardev_del:

chardev-del


> +Arguments:
> +
> +- "id": the chardev's ID, must be unique (json-string)
> +- "backend": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)

Given this line...

> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +
> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",

...this line is wrong, since "on" is not a json-bool.  It would have to
be "server":true

> +                                              "wait"    : "off" } }

Similar for "wait":false

-- 
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: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
  2012-10-15 14:22   ` Anthony Liguori
  2012-10-15 18:04   ` Eric Blake
@ 2012-10-16  9:13   ` Lei Li
  2012-10-17  9:27     ` Gerd Hoffmann
  2012-10-17  1:28   ` Luiz Capitulino
  3 siblings, 1 reply; 22+ messages in thread
From: Lei Li @ 2012-10-16  9:13 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On 10/15/2012 04:06 PM, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_del monitor commands.
>
> They work simliar to the netdev_{add,del} commands.  The hmp version of
> chardev_add accepts like the -chardev command line option does.  The qmp
> version expects the arguments being passed as named parameters.
>
> chardev_del just takes an id argument and zaps the chardev specified.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>   hmp.c            |   23 ++++++++++++++++++++
>   hmp.h            |    2 +
>   qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>   qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
>   qemu-char.h      |    1 +
>   qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 202 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
>   ETEXI
>
>       {
> +        .name       = "chardev_add",
> +        .args_type  = "args:s",
> +        .params     = "args",
> +        .help       = "add chardev",
> +        .mhandler.cmd = hmp_chardev_add,
> +    },
> +
> +STEXI
> +@item chardev_add args
> +@findex chardev_add
> +
> +chardev_add accepts the same parameters as the -chardev command line switch.
> +
> +ETEXI
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "id:s",
> +        .params     = "id",
> +        .help       = "del chardev",
> +        .mhandler.cmd = hmp_chardev_del,
> +    },
> +
> +STEXI
> +@item chardev_del id
> +@findex chardev_del
> +
> +Removes the chardev @var{id}.
> +
> +ETEXI
> +
> +    {
>           .name       = "info",
>           .args_type  = "item:s?",
>           .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 70bdec2..96bb900 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>       qmp_screendump(filename, &err);
>       hmp_handle_error(mon, &err);
>   }
> +
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
> +{
> +    const char *args = qdict_get_str(qdict, "args");
> +    Error *err = NULL;
> +    QemuOpts *opts;
> +
> +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
> +    if (opts == NULL) {
> +        error_setg(&err, "Parsing chardev args failed\n");
> +    } else {
> +        qemu_chr_new_from_opts(opts, NULL, &err);
> +    }
> +    hmp_handle_error(mon, &err);
> +}
> +
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    qmp_chardev_del(qdict_get_str(qdict, "id"),
> +                    &err);
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 71ea384..080afaa 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>   void hmp_closefd(Monitor *mon, const QDict *qdict);
>   void hmp_send_key(Monitor *mon, const QDict *qdict);
>   void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict);
>
>   #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f9dbdae..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>   # Since: 0.14.0
>   ##
>   { 'command': 'screendump', 'data': {'filename': 'str'} }
> +
> +##
> +# @chardev_add:
> +#
> +# Add a chardev
> +#
> +# @id: the chardev's ID, must be unique
> +# @backend: the chardev backend: "file", "socket", ...
> +# @path: file / device / unix socket path
> +# @name: spice channel name
> +# @host: host name
> +# @port: port number
> +# @server: create socket in server mode
> +# @wait: wait for connect
> +# @ipv4: force ipv4-only
> +# @ipv6: force ipv6-only
> +# @telnet: telnet negotiation
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @chardev_del:
> +#
> +# Remove a chardev
> +#
> +# @id: the chardev's ID, must exist and not be in use
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_del', 'data': {'id': 'str'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index be4ec61..dae3e3c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
>       return serial_hds[next_serial++];
>   }
>
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +void qmp_chardev_del(const char *id, Error **errp)
> +{
> +    CharDriverState *chr;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL == chr) {
> +        error_setg(errp, "Chardev '%s' not found\n", id);
> +        return;
> +    }
> +    if (chr->chr_can_read || chr->chr_read ||
> +        chr->chr_event || chr->handler_opaque) {
> +        error_setg(errp, "Chardev '%s' is busy\n", id);
> +        return;
> +    }
> +    qemu_chr_delete(chr);
> +}
> diff --git a/qemu-char.h b/qemu-char.h
> index 99bc132..407e0fe 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -238,6 +238,7 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
>   CharDriverState *qemu_chr_find(const char *name);
>
>   QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>
>   /* add an eventfd to the qemu devices that are polled */
>   CharDriverState *qemu_chr_open_eventfd(int eventfd);
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..ebc1cee 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2576,3 +2576,64 @@ EQMP
>           .args_type  = "",
>           .mhandler.cmd_new = qmp_marshal_input_query_target,
>       },
> +
> +    {
> +        .name       = "chardev_add",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_chardev_add,
> +    },
> +
> +SQMP
> +chardev_add
> +-----------
> +
> +Add a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must be unique (json-string)
> +- "backend": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)
> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +

As mentioned in another thread, should it support the "mux" argument
for multiplexing mode?

> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",
> +                                              "wait"    : "off" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
> +    },
> +
> +
> +SQMP
> +chardev_del
> +-----------
> +
> +Remove a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must exist and not be in use (json-string)
> +
> +Example:
> +
> +-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
> +<- { "return": {} }
> +
> +EQMP


-- 
Lei

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

* Re: [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann
@ 2012-10-17  1:12   ` Luiz Capitulino
  2012-10-17  1:13     ` Luiz Capitulino
  0 siblings, 1 reply; 22+ messages in thread
From: Luiz Capitulino @ 2012-10-17  1:12 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Mon, 15 Oct 2012 10:06:55 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-char.c |   24 +++++++++++++++---------
>  qemu-char.h |    3 ++-
>  vl.c        |    7 ++++++-
>  3 files changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index b082bae..e2e1da8 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2755,19 +2755,20 @@ static const struct {
>  };
>  
>  CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> -                                    void (*init)(struct CharDriverState *s))
> +                                    void (*init)(struct CharDriverState *s),
> +                                    Error **errp)
>  {
>      CharDriverState *chr;
>      int i;
>  
>      if (qemu_opts_id(opts) == NULL) {
> -        fprintf(stderr, "chardev: no id specified\n");
> +        error_setg(errp, "chardev: no id specified\n");
>          return NULL;
>      }
>  
>      if (qemu_opt_get(opts, "backend") == NULL) {
> -        fprintf(stderr, "chardev: \"%s\" missing backend\n",
> -                qemu_opts_id(opts));
> +        error_setg(errp, "chardev: \"%s\" missing backend\n",
> +                   qemu_opts_id(opts));
>          return NULL;
>      }
>      for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
> @@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>              break;
>      }
>      if (i == ARRAY_SIZE(backend_table)) {
> -        fprintf(stderr, "chardev: backend \"%s\" not found\n",
> -                qemu_opt_get(opts, "backend"));
> +        error_setg(errp, "chardev: backend \"%s\" not found\n",
> +                   qemu_opt_get(opts, "backend"));
>          return NULL;
>      }
>  
>      chr = backend_table[i].open(opts);
>      if (!chr) {
> -        fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
> -                qemu_opt_get(opts, "backend"));
> +        error_setg(errp, "chardev: opening backend \"%s\" failed\n",
> +                   qemu_opt_get(opts, "backend"));
>          return NULL;
>      }
>  
> @@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
>      const char *p;
>      CharDriverState *chr;
>      QemuOpts *opts;
> +    Error *err = NULL;
>  
>      if (strstart(filename, "chardev:", &p)) {
>          return qemu_chr_find(p);
> @@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
>      if (!opts)
>          return NULL;
>  
> -    chr = qemu_chr_new_from_opts(opts, init);
> +    chr = qemu_chr_new_from_opts(opts, init, &err);
> +    if (error_is_set(&err)) {
> +        fprintf(stderr, "%s\n", error_get_pretty(err));
> +        error_free(err);
> +    }
>      if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
>          monitor_init(chr, MONITOR_USE_READLINE);
>      }
> diff --git a/qemu-char.h b/qemu-char.h
> index 486644b..adae13e 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -88,7 +88,8 @@ struct CharDriverState {
>   * Returns: a new character backend
>   */
>  CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> -                                    void (*init)(struct CharDriverState *s));
> +                                    void (*init)(struct CharDriverState *s),
> +                                    Error **errp);
>  
>  /**
>   * @qemu_chr_new:
> diff --git a/vl.c b/vl.c
> index 5b357a3..35eb627 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1940,8 +1940,13 @@ static int device_init_func(QemuOpts *opts, void *opaque)
>  static int chardev_init_func(QemuOpts *opts, void *opaque)
>  {
>      CharDriverState *chr;
> +    Error *err = NULL;
>  
> -    chr = qemu_chr_new_from_opts(opts, NULL);
> +    chr = qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        fprintf(stderr, "%s\n", error_get_pretty(err));
> +        error_free(err);
> +    }
>      if (!chr)
>          return -1;

If you respin, please drop that check as it's not needed. Also, we usually
name the "internal" Error variable local_err.

>      return 0;

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

* Re: [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts
  2012-10-17  1:12   ` Luiz Capitulino
@ 2012-10-17  1:13     ` Luiz Capitulino
  0 siblings, 0 replies; 22+ messages in thread
From: Luiz Capitulino @ 2012-10-17  1:13 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Tue, 16 Oct 2012 22:12:13 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Mon, 15 Oct 2012 10:06:55 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  qemu-char.c |   24 +++++++++++++++---------
> >  qemu-char.h |    3 ++-
> >  vl.c        |    7 ++++++-
> >  3 files changed, 23 insertions(+), 11 deletions(-)
> > 
> > diff --git a/qemu-char.c b/qemu-char.c
> > index b082bae..e2e1da8 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -2755,19 +2755,20 @@ static const struct {
> >  };
> >  
> >  CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> > -                                    void (*init)(struct CharDriverState *s))
> > +                                    void (*init)(struct CharDriverState *s),
> > +                                    Error **errp)
> >  {
> >      CharDriverState *chr;
> >      int i;
> >  
> >      if (qemu_opts_id(opts) == NULL) {
> > -        fprintf(stderr, "chardev: no id specified\n");
> > +        error_setg(errp, "chardev: no id specified\n");
> >          return NULL;
> >      }
> >  
> >      if (qemu_opt_get(opts, "backend") == NULL) {
> > -        fprintf(stderr, "chardev: \"%s\" missing backend\n",
> > -                qemu_opts_id(opts));
> > +        error_setg(errp, "chardev: \"%s\" missing backend\n",
> > +                   qemu_opts_id(opts));
> >          return NULL;
> >      }
> >      for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
> > @@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> >              break;
> >      }
> >      if (i == ARRAY_SIZE(backend_table)) {
> > -        fprintf(stderr, "chardev: backend \"%s\" not found\n",
> > -                qemu_opt_get(opts, "backend"));
> > +        error_setg(errp, "chardev: backend \"%s\" not found\n",
> > +                   qemu_opt_get(opts, "backend"));
> >          return NULL;
> >      }
> >  
> >      chr = backend_table[i].open(opts);
> >      if (!chr) {
> > -        fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
> > -                qemu_opt_get(opts, "backend"));
> > +        error_setg(errp, "chardev: opening backend \"%s\" failed\n",
> > +                   qemu_opt_get(opts, "backend"));
> >          return NULL;
> >      }
> >  
> > @@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
> >      const char *p;
> >      CharDriverState *chr;
> >      QemuOpts *opts;
> > +    Error *err = NULL;
> >  
> >      if (strstart(filename, "chardev:", &p)) {
> >          return qemu_chr_find(p);
> > @@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
> >      if (!opts)
> >          return NULL;
> >  
> > -    chr = qemu_chr_new_from_opts(opts, init);
> > +    chr = qemu_chr_new_from_opts(opts, init, &err);
> > +    if (error_is_set(&err)) {
> > +        fprintf(stderr, "%s\n", error_get_pretty(err));
> > +        error_free(err);
> > +    }
> >      if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
> >          monitor_init(chr, MONITOR_USE_READLINE);
> >      }
> > diff --git a/qemu-char.h b/qemu-char.h
> > index 486644b..adae13e 100644
> > --- a/qemu-char.h
> > +++ b/qemu-char.h
> > @@ -88,7 +88,8 @@ struct CharDriverState {
> >   * Returns: a new character backend
> >   */
> >  CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> > -                                    void (*init)(struct CharDriverState *s));
> > +                                    void (*init)(struct CharDriverState *s),
> > +                                    Error **errp);
> >  
> >  /**
> >   * @qemu_chr_new:
> > diff --git a/vl.c b/vl.c
> > index 5b357a3..35eb627 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1940,8 +1940,13 @@ static int device_init_func(QemuOpts *opts, void *opaque)
> >  static int chardev_init_func(QemuOpts *opts, void *opaque)
> >  {
> >      CharDriverState *chr;
> > +    Error *err = NULL;
> >  
> > -    chr = qemu_chr_new_from_opts(opts, NULL);
> > +    chr = qemu_chr_new_from_opts(opts, NULL, &err);
> > +    if (error_is_set(&err)) {
> > +        fprintf(stderr, "%s\n", error_get_pretty(err));
> > +        error_free(err);
> > +    }
> >      if (!chr)
> >          return -1;
> 
> If you respin, please drop that check as it's not needed. Also, we usually
> name the "internal" Error variable local_err.

Forgot to say that patch looks good to me.

> 
> >      return 0;
> 

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

* Re: [Qemu-devel] [PATCH v3 8/9] chardev: fix QemuOpts lifecycle
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 8/9] chardev: fix QemuOpts lifecycle Gerd Hoffmann
@ 2012-10-17  1:18   ` Luiz Capitulino
  0 siblings, 0 replies; 22+ messages in thread
From: Luiz Capitulino @ 2012-10-17  1:18 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Mon, 15 Oct 2012 10:06:56 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> qemu_chr_new_from_opts handles QemuOpts release now, so callers don't
> have to worry.  It will either be saved in CharDriverState, then
> released in qemu_chr_delete, or in the error case released instantly.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

A comment in qemu_chr_new_from_opts() saying that it takes ownership of
opts would be nice though.

> ---
>  qemu-char.c |   15 ++++++++++-----
>  qemu-char.h |    1 +
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index e2e1da8..be4ec61 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2763,13 +2763,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>  
>      if (qemu_opts_id(opts) == NULL) {
>          error_setg(errp, "chardev: no id specified\n");
> -        return NULL;
> +        goto err;
>      }
>  
>      if (qemu_opt_get(opts, "backend") == NULL) {
>          error_setg(errp, "chardev: \"%s\" missing backend\n",
>                     qemu_opts_id(opts));
> -        return NULL;
> +        goto err;
>      }
>      for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
>          if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0)
> @@ -2778,14 +2778,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>      if (i == ARRAY_SIZE(backend_table)) {
>          error_setg(errp, "chardev: backend \"%s\" not found\n",
>                     qemu_opt_get(opts, "backend"));
> -        return NULL;
> +        goto err;
>      }
>  
>      chr = backend_table[i].open(opts);
>      if (!chr) {
>          error_setg(errp, "chardev: opening backend \"%s\" failed\n",
>                     qemu_opt_get(opts, "backend"));
> -        return NULL;
> +        goto err;
>      }
>  
>      if (!chr->filename)
> @@ -2806,7 +2806,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>          chr->avail_connections = 1;
>      }
>      chr->label = g_strdup(qemu_opts_id(opts));
> +    chr->opts = opts;
>      return chr;
> +
> +err:
> +    qemu_opts_del(opts);
> +    return NULL;
>  }
>  
>  CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s))
> @@ -2832,7 +2837,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
>      if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
>          monitor_init(chr, MONITOR_USE_READLINE);
>      }
> -    qemu_opts_del(opts);
>      return chr;
>  }
>  
> @@ -2864,6 +2868,7 @@ void qemu_chr_delete(CharDriverState *chr)
>          chr->chr_close(chr);
>      g_free(chr->filename);
>      g_free(chr->label);
> +    qemu_opts_del(chr->opts);
>      g_free(chr);
>  }
>  
> diff --git a/qemu-char.h b/qemu-char.h
> index adae13e..99bc132 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -74,6 +74,7 @@ struct CharDriverState {
>      char *filename;
>      int opened;
>      int avail_connections;
> +    QemuOpts *opts;
>      QTAILQ_ENTRY(CharDriverState) next;
>  };
>  

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

* Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
                     ` (2 preceding siblings ...)
  2012-10-16  9:13   ` Lei Li
@ 2012-10-17  1:28   ` Luiz Capitulino
  3 siblings, 0 replies; 22+ messages in thread
From: Luiz Capitulino @ 2012-10-17  1:28 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Mon, 15 Oct 2012 10:06:57 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> This patch adds chardev_add and chardev_del monitor commands.
> 
> They work simliar to the netdev_{add,del} commands.  The hmp version of
> chardev_add accepts like the -chardev command line option does.  The qmp
> version expects the arguments being passed as named parameters.
> 
> chardev_del just takes an id argument and zaps the chardev specified.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>  hmp.c            |   23 ++++++++++++++++++++
>  hmp.h            |    2 +
>  qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>  qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
>  qemu-char.h      |    1 +
>  qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 202 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
>  ETEXI
>  
>      {
> +        .name       = "chardev_add",
> +        .args_type  = "args:s",
> +        .params     = "args",
> +        .help       = "add chardev",
> +        .mhandler.cmd = hmp_chardev_add,
> +    },
> +
> +STEXI
> +@item chardev_add args
> +@findex chardev_add
> +
> +chardev_add accepts the same parameters as the -chardev command line switch.
> +
> +ETEXI
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "id:s",
> +        .params     = "id",
> +        .help       = "del chardev",
> +        .mhandler.cmd = hmp_chardev_del,
> +    },
> +
> +STEXI
> +@item chardev_del id
> +@findex chardev_del
> +
> +Removes the chardev @var{id}.
> +
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 70bdec2..96bb900 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>      qmp_screendump(filename, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
> +{
> +    const char *args = qdict_get_str(qdict, "args");
> +    Error *err = NULL;
> +    QemuOpts *opts;
> +
> +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
> +    if (opts == NULL) {
> +        error_setg(&err, "Parsing chardev args failed\n");

You can monitor_printf() directly if you want.

> +    } else {
> +        qemu_chr_new_from_opts(opts, NULL, &err);
> +    }
> +    hmp_handle_error(mon, &err);
> +}
> +
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    qmp_chardev_del(qdict_get_str(qdict, "id"),
> +                    &err);
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 71ea384..080afaa 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>  void hmp_closefd(Monitor *mon, const QDict *qdict);
>  void hmp_send_key(Monitor *mon, const QDict *qdict);
>  void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f9dbdae..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'screendump', 'data': {'filename': 'str'} }
> +
> +##
> +# @chardev_add:
> +#
> +# Add a chardev
> +#
> +# @id: the chardev's ID, must be unique
> +# @backend: the chardev backend: "file", "socket", ...

I agree with Eric's comment that this should be an enum.

> +# @path: file / device / unix socket path
> +# @name: spice channel name
> +# @host: host name
> +# @port: port number
> +# @server: create socket in server mode
> +# @wait: wait for connect
> +# @ipv4: force ipv4-only
> +# @ipv6: force ipv6-only
> +# @telnet: telnet negotiation
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @chardev_del:
> +#
> +# Remove a chardev
> +#
> +# @id: the chardev's ID, must exist and not be in use
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_del', 'data': {'id': 'str'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index be4ec61..dae3e3c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
>      return serial_hds[next_serial++];
>  }
>  
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +void qmp_chardev_del(const char *id, Error **errp)
> +{
> +    CharDriverState *chr;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL == chr) {
> +        error_setg(errp, "Chardev '%s' not found\n", id);
> +        return;
> +    }
> +    if (chr->chr_can_read || chr->chr_read ||
> +        chr->chr_event || chr->handler_opaque) {
> +        error_setg(errp, "Chardev '%s' is busy\n", id);
> +        return;
> +    }
> +    qemu_chr_delete(chr);
> +}
> diff --git a/qemu-char.h b/qemu-char.h
> index 99bc132..407e0fe 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -238,6 +238,7 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
>  CharDriverState *qemu_chr_find(const char *name);
>  
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>  
>  /* add an eventfd to the qemu devices that are polled */
>  CharDriverState *qemu_chr_open_eventfd(int eventfd);
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..ebc1cee 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2576,3 +2576,64 @@ EQMP
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_input_query_target,
>      },
> +
> +    {
> +        .name       = "chardev_add",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_chardev_add,
> +    },
> +
> +SQMP
> +chardev_add
> +-----------
> +
> +Add a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must be unique (json-string)
> +- "backend": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)
> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +
> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",
> +                                              "wait"    : "off" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
> +    },
> +
> +
> +SQMP
> +chardev_del
> +-----------
> +
> +Remove a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must exist and not be in use (json-string)
> +
> +Example:
> +
> +-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
> +<- { "return": {} }
> +
> +EQMP

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

* Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-15 18:04   ` Eric Blake
@ 2012-10-17  1:49     ` Luiz Capitulino
  0 siblings, 0 replies; 22+ messages in thread
From: Luiz Capitulino @ 2012-10-17  1:49 UTC (permalink / raw)
  To: Eric Blake; +Cc: Gerd Hoffmann, qemu-devel

On Mon, 15 Oct 2012 12:04:52 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 10/15/2012 02:06 AM, Gerd Hoffmann wrote:
> > This patch adds chardev_add and chardev_del monitor commands.
> > 
> > They work simliar to the netdev_{add,del} commands.  The hmp version of
> 
> s/simliar/similar/
> 
> > chardev_add accepts like the -chardev command line option does.  The qmp
> > version expects the arguments being passed as named parameters.
> > 
> > chardev_del just takes an id argument and zaps the chardev specified.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > +++ b/qapi-schema.json
> > @@ -2796,3 +2796,42 @@
> >  # Since: 0.14.0
> >  ##
> >  { 'command': 'screendump', 'data': {'filename': 'str'} }
> > +
> > +##
> > +# @chardev_add:
> 
> chardev-add
> 
> > +#
> > +# Add a chardev
> > +#
> > +# @id: the chardev's ID, must be unique
> > +# @backend: the chardev backend: "file", "socket", ...
> 
> Should this be an enum type, instead of an open-coded string?
> 
> > +# @path: file / device / unix socket path
> > +# @name: spice channel name
> > +# @host: host name
> > +# @port: port number
> > +# @server: create socket in server mode
> > +# @wait: wait for connect
> > +# @ipv4: force ipv4-only
> > +# @ipv6: force ipv6-only
> > +# @telnet: telnet negotiation
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 1.3.0
> > +##
> > +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> > +                                     'backend' : 'str',
> > +                                     '*props'  : '**' },
> 
> Having an open-coded list for props feels awkward; it would be nicer to
> have the schema completely describe everything, even though that may be
> more documentation work.

I agree it's awkward. Besides, this 'props' thing was supposed to be only used
with old commands that already accept QemuOpts style options. The usage of
'props' forces us having a front-end in old qmp style, which we don't want to
get spread.

Having said that, I'm not sure I can offer a good alternative here. Having
all options in the schema would require us to create the QemuOpts object by
hand vs. automatically building it from the qdict that's passed to old
style qmp commands. It's not only documentation work.

There's the work Laszlo did for net clients too. Where we have a
NetClientOptions type which is a union of all possible net clients
options types. All clients get a NetClientOptions instance. But this would
require massive conversion of chardev backends to use a similar type.

Another idea would be to have a different add command for each backend.
This is more work and more "verbose", but has the good effect of a clean
separation & definition of the set of options used by each backend.

But again, as I'm not sure any of my ideas above are good, I'd be fine
with the chardev_add command from this patch.

> 
> > +  'gen': 'no' }
> > +
> > +##
> > +# @chardev_del:
> 
> chardev-del
> 
> 
> > +Arguments:
> > +
> > +- "id": the chardev's ID, must be unique (json-string)
> > +- "backend": the chardev backend: "file", "socket", ... (json-string)
> > +- "path": file / device / unix socket path (json-string, optional)
> > +- "name": spice channel name (json-string, optional)
> > +- "host": host name (json-string, optional)
> > +- "port": port number (json-string, optional)
> > +- "server": create socket in server mode (json-bool, optional)
> 
> Given this line...
> 
> > +- "wait": wait for connect (json-bool, optional)
> > +- "ipv4": force ipv4-only (json-bool, optional)
> > +- "ipv6": force ipv6-only (json-bool, optional)
> > +- "telnet": telnet negotiation (json-bool, optional)
> > +
> > +Example:
> > +
> > +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> > +                                              "backend" : "socket",
> > +                                              "path"    : "/tmp/foo",
> > +                                              "server"  : "on",
> 
> ...this line is wrong, since "on" is not a json-bool.  It would have to
> be "server":true
> 
> > +                                              "wait"    : "off" } }
> 
> Similar for "wait":false
> 

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

* Re: [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series.
  2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
                   ` (8 preceding siblings ...)
  2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
@ 2012-10-17  1:52 ` Luiz Capitulino
  9 siblings, 0 replies; 22+ messages in thread
From: Luiz Capitulino @ 2012-10-17  1:52 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Mon, 15 Oct 2012 10:06:48 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> This patch series tackles serial device hotplug.

This conflicts with Paolo's work merged in:

 git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

You could base your work on top of that to avoid conflicts, but note that
I might rebase that branch.

> 
> The first four patches have been on the list before, they implement
> pci-serial devices featuring a hot-pluggable 16550 uart and got some
> minor tweaks only.
> 
> The next two patches update the usb-serial device.  It will only show up
> in the guest when the chardev is open.  You'll see the difference with
> socket chardevs:  If you open the chardev (by connecting to the socket)
> the device will show up in the guest, on close (disconnect) it will
> disappear.
> 
> Final three patches cleanup chardev a bit and adds chardev hotplug to
> the mix, which makes the other patches alot more useful.  It is the
> missing bit needed to really hotplug serial devices:
> 
>    (qemu) chardev_add file,id=pciserial,path=/root/hotchardev.log
>    (qemu) device_add pci-serial,id=pciserial,chardev=pciserial
> 
> And the reverse:
> 
>    (qemu) device_del pciserial
>    (qemu) chardev_del pciserial
> 
> New in v2:
>  - added two chardev cleanup patches.
>  - switched chardev_{add,del} commands to netdev_{add,del} style.
> 
> cheers,
>   Gerd
> 
> The following changes since commit 8b4a3df8081f3e6f1061ed5cbb303ad623ade66b:
> 
>   Fix popcnt in long mode (2012-10-14 14:55:09 +0400)
> 
> are available in the git repository at:
>   git://git.kraxel.org/qemu serial.2
> 
> Gerd Hoffmann (9):
>       serial: split serial.c
>       serial: add pci variant
>       serial: add windows inf file for the pci card to docs
>       serial: add 2x + 4x pci variant
>       usb-serial: don't magically zap chardev on umplug
>       usb-serial: only expose device in guest when the chardev is open
>       chardev: add error reporting for qemu_chr_new_from_opts
>       chardev: fix QemuOpts lifecycle
>       chardev: add hotplug support.
> 
>  default-configs/pci.mak  |    2 +
>  docs/qemupciserial.inf   |  109 ++++++++++++++++++
>  hmp-commands.hx          |   32 ++++++
>  hmp.c                    |   23 ++++
>  hmp.h                    |    2 +
>  hw/Makefile.objs         |    3 +-
>  hw/alpha_dp264.c         |    1 +
>  hw/kzm.c                 |    2 +-
>  hw/mips_fulong2e.c       |    1 +
>  hw/mips_jazz.c           |    1 +
>  hw/mips_malta.c          |    1 +
>  hw/mips_mipssim.c        |    2 +-
>  hw/mips_r4k.c            |    1 +
>  hw/musicpal.c            |    2 +-
>  hw/omap_uart.c           |    3 +-
>  hw/openrisc_sim.c        |    3 +-
>  hw/pc.c                  |    1 +
>  hw/pc.h                  |   27 -----
>  hw/pci_ids.h             |    1 +
>  hw/petalogix_ml605_mmu.c |    2 +-
>  hw/ppc/e500.c            |    2 +-
>  hw/ppc405_uc.c           |    2 +-
>  hw/ppc440_bamboo.c       |    2 +-
>  hw/ppc_prep.c            |    1 +
>  hw/pxa2xx.c              |    2 +-
>  hw/serial-isa.c          |  130 ++++++++++++++++++++++
>  hw/serial-pci.c          |  272 ++++++++++++++++++++++++++++++++++++++++++++++
>  hw/serial.c              |  149 ++-----------------------
>  hw/serial.h              |   74 +++++++++++++
>  hw/sm501.c               |    2 +-
>  hw/sun4u.c               |    1 +
>  hw/usb/dev-serial.c      |   21 +++-
>  hw/virtex_ml507.c        |    2 +-
>  hw/xtensa_lx60.c         |    3 +-
>  qapi-schema.json         |   39 +++++++
>  qemu-char.c              |   83 ++++++++++++---
>  qemu-char.h              |    5 +-
>  qmp-commands.hx          |   61 ++++++++++
>  vl.c                     |    7 +-
>  39 files changed, 878 insertions(+), 199 deletions(-)
>  create mode 100644 docs/qemupciserial.inf
>  create mode 100644 hw/serial-isa.c
>  create mode 100644 hw/serial-pci.c
>  create mode 100644 hw/serial.h
> 

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

* Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
  2012-10-16  9:13   ` Lei Li
@ 2012-10-17  9:27     ` Gerd Hoffmann
  0 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2012-10-17  9:27 UTC (permalink / raw)
  To: Lei Li; +Cc: qemu-devel

  Hi,

>> +- "id": the chardev's ID, must be unique (json-string)
>> +- "backend": the chardev backend: "file", "socket", ... (json-string)
>> +- "path": file / device / unix socket path (json-string, optional)
>> +- "name": spice channel name (json-string, optional)
>> +- "host": host name (json-string, optional)
>> +- "port": port number (json-string, optional)
>> +- "server": create socket in server mode (json-bool, optional)
>> +- "wait": wait for connect (json-bool, optional)
>> +- "ipv4": force ipv4-only (json-bool, optional)
>> +- "ipv6": force ipv6-only (json-bool, optional)
>> +- "telnet": telnet negotiation (json-bool, optional)
>> +
> 
> As mentioned in another thread, should it support the "mux" argument
> for multiplexing mode?

I don't think so.  Basically the only use case for the mux chardev is
-nographic with serial line + monitor being multiplexed on stdio.

cheers,
  Gerd

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

end of thread, other threads:[~2012-10-17  9:27 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-15  8:06 [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Gerd Hoffmann
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 1/9] serial: split serial.c Gerd Hoffmann
2012-10-15 14:16   ` Anthony Liguori
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 2/9] serial: add pci variant Gerd Hoffmann
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 3/9] serial: add windows inf file for the pci card to docs Gerd Hoffmann
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 4/9] serial: add 2x + 4x pci variant Gerd Hoffmann
2012-10-15 14:18   ` Anthony Liguori
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 5/9] usb-serial: don't magically zap chardev on umplug Gerd Hoffmann
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 6/9] usb-serial: only expose device in guest when the chardev is open Gerd Hoffmann
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 7/9] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann
2012-10-17  1:12   ` Luiz Capitulino
2012-10-17  1:13     ` Luiz Capitulino
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 8/9] chardev: fix QemuOpts lifecycle Gerd Hoffmann
2012-10-17  1:18   ` Luiz Capitulino
2012-10-15  8:06 ` [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support Gerd Hoffmann
2012-10-15 14:22   ` Anthony Liguori
2012-10-15 18:04   ` Eric Blake
2012-10-17  1:49     ` Luiz Capitulino
2012-10-16  9:13   ` Lei Li
2012-10-17  9:27     ` Gerd Hoffmann
2012-10-17  1:28   ` Luiz Capitulino
2012-10-17  1:52 ` [Qemu-devel] [PATCH v3 0/9] serial device hotplug patch series Luiz Capitulino

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.