[Qemu-devel] [PATCH 0/2] add pci-serial device.

[Qemu-devel] [PATCH 0/2] add pci-serial device.

[Gerd Hoffmann]

  Hi,

Two patches, first split up serial.c a bit,
then actually add the pci-based serial device.

cheers,
  Gerd

Gerd Hoffmann (2):
  serial: split serial.c
  serial: add pci variant

 default-configs/pci.mak  |    2 +
 docs/pciserial.inf       |   96 +++++++++++++++++++++++++++++++
 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          |  101 ++++++++++++++++++++++++++++++++
 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 +-
 30 files changed, 433 insertions(+), 180 deletions(-)
 create mode 100644 docs/pciserial.inf
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial-pci.c
 create mode 100644 hw/serial.h

[Qemu-devel] [PATCH 1/2] serial: split serial.c

[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 6dfebd2..7a27889 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -20,7 +20,7 @@ hw-obj-$(CONFIG_M48T59) += m48t59.o
 hw-obj-$(CONFIG_ESCC) += escc.o
 hw-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
-hw-obj-$(CONFIG_SERIAL) += serial.o
+hw-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
 hw-obj-$(CONFIG_PARALLEL) += parallel.o
 hw-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 hw-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
index 9eb939f..faeb275 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 38e4b86..8a38cd9 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 ad23f26..05a1eaa 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 7e7e0e2..f056777 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 e4db071..170a265 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 dced648..03f68e8 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 "microblaze_boot.h"
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 6f0de6d..466435a 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 89e5013..3d3c33c 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 592b7b2..0390800 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 07cd042..2a85ddb 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

[Qemu-devel] [PATCH 2/2] serial: add pci variant

[Gerd Hoffmann]

So we get a hot-pluggable 16550 uart.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 default-configs/pci.mak |    2 +
 docs/pciserial.inf      |   96 ++++++++++++++++++++++++++++++++++++++++++++
 hw/Makefile.objs        |    1 +
 hw/pci_ids.h            |    1 +
 hw/serial-pci.c         |  101 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 201 insertions(+), 0 deletions(-)
 create mode 100644 docs/pciserial.inf
 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/docs/pciserial.inf b/docs/pciserial.inf
new file mode 100644
index 0000000..76e8d9d
--- /dev/null
+++ b/docs/pciserial.inf
@@ -0,0 +1,96 @@
+; pciserial.inf for qemu, based on MSPORTS.INF
+
+[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"
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 7a27889..9ab8878 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -21,6 +21,7 @@ hw-obj-$(CONFIG_ESCC) += escc.o
 hw-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
 hw-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
+hw-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
 hw-obj-$(CONFIG_PARALLEL) += parallel.o
 hw-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 hw-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..68401f0
--- /dev/null
+++ b/hw/serial-pci.c
@@ -0,0 +1,101 @@
+/*
+ * 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 "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;
+
+    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL);
+    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)
-- 
1.7.1

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Anthony Liguori]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> Two patches, first split up serial.c a bit,
> then actually add the pci-based serial device.

The series looks good to me.  A couple requests:

1) Could you add a spec describing this new PCI device?  Doesn't need to
   be more than a couple paragraphs since the device is super simple.

2) Could you make the inf file an separate patch and either include
   documentation in the commit message on how to use it with Windows or
   just add a comment to the inf file?

This is a new PCI space for QEMU too.  Is this a driver that is "owned"
by QEMU and Red Hat is donating the PCI id or is this a driver that RH
controls that we're implementing?

The only reason I ask is whether this is something we can add new
features to.  I can't think of one off hand, but it can't hurt to work
this out up front.

Regards,

Anthony Liguori

>
> cheers,
>   Gerd
>
> Gerd Hoffmann (2):
>   serial: split serial.c
>   serial: add pci variant
>
>  default-configs/pci.mak  |    2 +
>  docs/pciserial.inf       |   96 +++++++++++++++++++++++++++++++
>  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          |  101 ++++++++++++++++++++++++++++++++
>  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 +-
>  30 files changed, 433 insertions(+), 180 deletions(-)
>  create mode 100644 docs/pciserial.inf
>  create mode 100644 hw/serial-isa.c
>  create mode 100644 hw/serial-pci.c
>  create mode 100644 hw/serial.h

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Jan Kiszka]

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

On 2012-09-26 01:43, Anthony Liguori wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
>>   Hi,
>>
>> Two patches, first split up serial.c a bit,
>> then actually add the pci-based serial device.
> 
> The series looks good to me.  A couple requests:
> 
> 1) Could you add a spec describing this new PCI device?  Doesn't need to
>    be more than a couple paragraphs since the device is super simple.
> 
> 2) Could you make the inf file an separate patch and either include
>    documentation in the commit message on how to use it with Windows or
>    just add a comment to the inf file?
> 
> This is a new PCI space for QEMU too.  Is this a driver that is "owned"
> by QEMU and Red Hat is donating the PCI id or is this a driver that RH
> controls that we're implementing?
> 
> The only reason I ask is whether this is something we can add new
> features to.  I can't think of one off hand, but it can't hurt to work
> this out up front.

Multiport e.g. (to save PCI slots). There was some proposal recently to
add a model of an real multiport PCI card, just don't find the mail
right now...

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Gerd Hoffmann]

On 09/26/12 01:43, Anthony Liguori wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
>>   Hi,
>>
>> Two patches, first split up serial.c a bit,
>> then actually add the pci-based serial device.
> 
> The series looks good to me.  A couple requests:
> 
> 1) Could you add a spec describing this new PCI device?  Doesn't need to
>    be more than a couple paragraphs since the device is super simple.

Well, it is pretty strait forward:  A single IO bar, 8 bytes in size,
where the 16550 uart is mapped to:

[kraxel@fedora ~]$ lspci -vse
00:0e.0 Serial controller: Red Hat, Inc. Device 0002 (rev 01) (prog-if
00 [8250])
	Subsystem: Red Hat, Inc Device 1100
	Physical Slot: 14
	Flags: fast devsel, IRQ 11
	I/O ports at c130 [size=8]
	Kernel driver in use: serial

But I can surely add a comment about it.

> 2) Could you make the inf file an separate patch and either include
>    documentation in the commit message on how to use it with Windows or
>    just add a comment to the inf file?

I think a comment is better, easier to find than a commit message.  Will do.

> This is a new PCI space for QEMU too.  

It isn't new, I just followed what the pci bridge is doing (which has
1b36:0001).

> Is this a driver that is "owned"
> by QEMU and Red Hat is donating the PCI id or is this a driver that RH
> controls that we're implementing?

I consider it being owned by qemu.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Gerd Hoffmann]

  Hi,

>> The only reason I ask is whether this is something we can add
>> new features to.  I can't think of one off hand, but it can't
>> hurt to work this out up front.
> 
> Multiport e.g. (to save PCI slots). There was some proposal
> recently to add a model of an real multiport PCI card, just don't
> find the mail right now...

Easy enough to add.  Should be a separate device with its own pci id
though.  According to the linux source code there seem to be two
common ways to implement this:  A single, large pci bar where all
uarts are mapped one after the other.  Or one pci bar for each uart.
Now I need to figure which is easier to handle for windows guests ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Anthony Liguori]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On 09/26/12 01:43, Anthony Liguori wrote:
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>> 
>>>   Hi,
>>>
>>> Two patches, first split up serial.c a bit,
>>> then actually add the pci-based serial device.
>> 
>> The series looks good to me.  A couple requests:
>> 
>> 1) Could you add a spec describing this new PCI device?  Doesn't need to
>>    be more than a couple paragraphs since the device is super simple.
>
> Well, it is pretty strait forward:  A single IO bar, 8 bytes in size,
> where the 16550 uart is mapped to:
>
> [kraxel@fedora ~]$ lspci -vse
> 00:0e.0 Serial controller: Red Hat, Inc. Device 0002 (rev 01) (prog-if
> 00 [8250])
> 	Subsystem: Red Hat, Inc Device 1100
> 	Physical Slot: 14
> 	Flags: fast devsel, IRQ 11
> 	I/O ports at c130 [size=8]
> 	Kernel driver in use: serial
>
> But I can surely add a comment about it.

Understood, but I'd really prefer a file in docs/.  We should be
rigorous about having formal specs for all of our paravirtual devices.
The code shouldn't be the spec.

>
>> 2) Could you make the inf file an separate patch and either include
>>    documentation in the commit message on how to use it with Windows or
>>    just add a comment to the inf file?
>
> I think a comment is better, easier to find than a commit message.
> Will do.

So do I.  Thanks.

>> This is a new PCI space for QEMU too.  
>
> It isn't new, I just followed what the pci bridge is doing (which has
> 1b36:0001).

Ah, Michael, could you add a quick spec to docs for the pci_bridge
device?

>
>> Is this a driver that is "owned"
>> by QEMU and Red Hat is donating the PCI id or is this a driver that RH
>> controls that we're implementing?
>
> I consider it being owned by qemu.

Great.

Regards,

Anthony Liguori

>
> cheers,
>   Gerd

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Gerd Hoffmann]

  Hi,

> Understood, but I'd really prefer a file in docs/.  We should be
> rigorous about having formal specs for all of our paravirtual devices.
> The code shouldn't be the spec.

Well, pci-serial and pci-bridge are *not* paravirtual devices.

They follow a specification describing the programming interface, and
likewise does real hardware.  Same is true for all usb host controllers
and ahci btw.  I can certainly place a text file for pci-serial in
docs/spec/, but there isn't much qemu-specific to specify ...

Guests have generic drivers which just match the PCI class and
programming interface fields in the pci config space and don't care
(much) what the pci id is.  The pci id is used to print the name of the
hardware, apply quirks, handle vendor-specific extensions, and in case
of pci-serial windows also uses it to figure whenever the device is just
a serial port or a modem (behind a 16550).

Whenever we'll pick the pci ids of existing hardware or assign a unique
one is a matter of taste.  Picking unique IDs from Red Hat vendor space
doesn't make the devices paravirtual.  usb controllers and ahci got IDs
matching the ones of the intel chipsets (piix, q35) emulated by qemu,
which makes sense in that case.

For pci-serial windows needs a "driver" (which is just a inf file, the
driver itself is shipped by windows).  So in that case it is easier to
go with our own ids I think, as we can simply ship a inf file then.
When picking the IDs of other cards, existing as real hardware, users
would have to hunt down the (non-redistributable) driver package for the
real hardware to get it going in windows.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Michael S. Tsirkin]

On Thu, Sep 27, 2012 at 07:53:47AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > Understood, but I'd really prefer a file in docs/.  We should be
> > rigorous about having formal specs for all of our paravirtual devices.
> > The code shouldn't be the spec.
> 
> Well, pci-serial and pci-bridge are *not* paravirtual devices.
> 
> They follow a specification describing the programming interface, and
> likewise does real hardware.  Same is true for all usb host controllers
> and ahci btw.  I can certainly place a text file for pci-serial in
> docs/spec/, but there isn't much qemu-specific to specify ...
> 
> Guests have generic drivers which just match the PCI class and
> programming interface fields in the pci config space and don't care
> (much) what the pci id is.  The pci id is used to print the name of the
> hardware, apply quirks, handle vendor-specific extensions, and in case
> of pci-serial windows also uses it to figure whenever the device is just
> a serial port or a modem (behind a 16550).
> 
> Whenever we'll pick the pci ids of existing hardware or assign a unique
> one is a matter of taste.  Picking unique IDs from Red Hat vendor space
> doesn't make the devices paravirtual.  usb controllers and ahci got IDs
> matching the ones of the intel chipsets (piix, q35) emulated by qemu,
> which makes sense in that case.
> 
> For pci-serial windows needs a "driver" (which is just a inf file, the
> driver itself is shipped by windows).  So in that case it is easier to
> go with our own ids I think, as we can simply ship a inf file then.
> When picking the IDs of other cards, existing as real hardware, users
> would have to hunt down the (non-redistributable) driver package for the
> real hardware to get it going in windows.
> 
> cheers,
>   Gerd

Any way to bypass the need to distribute the inf?

-- 
MST

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Michael S. Tsirkin]

On Wed, Sep 26, 2012 at 08:01:39AM -0500, Anthony Liguori wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > On 09/26/12 01:43, Anthony Liguori wrote:
> >> Gerd Hoffmann <kraxel@redhat.com> writes:
> >> 
> >>>   Hi,
> >>>
> >>> Two patches, first split up serial.c a bit,
> >>> then actually add the pci-based serial device.
> >> 
> >> The series looks good to me.  A couple requests:
> >> 
> >> 1) Could you add a spec describing this new PCI device?  Doesn't need to
> >>    be more than a couple paragraphs since the device is super simple.
> >
> > Well, it is pretty strait forward:  A single IO bar, 8 bytes in size,
> > where the 16550 uart is mapped to:
> >
> > [kraxel@fedora ~]$ lspci -vse
> > 00:0e.0 Serial controller: Red Hat, Inc. Device 0002 (rev 01) (prog-if
> > 00 [8250])
> > 	Subsystem: Red Hat, Inc Device 1100
> > 	Physical Slot: 14
> > 	Flags: fast devsel, IRQ 11
> > 	I/O ports at c130 [size=8]
> > 	Kernel driver in use: serial
> >
> > But I can surely add a comment about it.
> 
> Understood, but I'd really prefer a file in docs/.  We should be
> rigorous about having formal specs for all of our paravirtual devices.
> The code shouldn't be the spec.
> 
> >
> >> 2) Could you make the inf file an separate patch and either include
> >>    documentation in the commit message on how to use it with Windows or
> >>    just add a comment to the inf file?
> >
> > I think a comment is better, easier to find than a commit message.
> > Will do.
> 
> So do I.  Thanks.
> 
> >> This is a new PCI space for QEMU too.  
> >
> > It isn't new, I just followed what the pci bridge is doing (which has
> > 1b36:0001).
> 
> Ah, Michael, could you add a quick spec to docs for the pci_bridge
> device?

What exactly would you like to see in this spec?

> >
> >> Is this a driver that is "owned"
> >> by QEMU and Red Hat is donating the PCI id or is this a driver that RH
> >> controls that we're implementing?
> >
> > I consider it being owned by qemu.
> 
> Great.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > cheers,
> >   Gerd

Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Gerd Hoffmann]

  Hi,

>> For pci-serial windows needs a "driver" (which is just a inf file, the
>> driver itself is shipped by windows).  So in that case it is easier to
>> go with our own ids I think, as we can simply ship a inf file then.
>> When picking the IDs of other cards, existing as real hardware, users
>> would have to hunt down the (non-redistributable) driver package for the
>> real hardware to get it going in windows.
>>
>> cheers,
>>   Gerd
> 
> Any way to bypass the need to distribute the inf?

The usual way to pick ids supported by windows out-of-the-box
unfortunaly doesn't fly for pci-serial cards.  I'm happy that we don't
have to code anything and just the inf file to hook up the serial.sys
shipped by windows is enougth.  I'm certainly open for better ideas
though ...

cheers,
  Gerd