qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy
@ 2011-02-03  6:17 Amit Shah
  2011-02-03  6:17 ` [Qemu-devel] [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control Amit Shah
  2011-02-07 15:46 ` [Qemu-devel] Re: [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy Alex Williamson
  0 siblings, 2 replies; 5+ messages in thread
From: Amit Shah @ 2011-02-03  6:17 UTC (permalink / raw)
  To: qemu list; +Cc: Amit Shah, Alex Williamson, Michael S. Tsirkin

Instead of using a single variable to pass to the virtio_serial_init
function, use a struct so that expanding the number of variables to be
passed on later is easier.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-pci.c        |   12 ++++++------
 hw/virtio-serial-bus.c |   16 ++++++++--------
 hw/virtio-serial.h     |    5 +++++
 hw/virtio.h            |    3 ++-
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 3911b09..952b5d2 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -18,6 +18,7 @@
 #include "virtio.h"
 #include "virtio-blk.h"
 #include "virtio-net.h"
+#include "virtio-serial.h"
 #include "pci.h"
 #include "qemu-error.h"
 #include "msix.h"
@@ -109,8 +110,7 @@ typedef struct {
 #ifdef CONFIG_LINUX
     V9fsConf fsconf;
 #endif
-    /* Max. number of ports we can have for a the virtio-serial device */
-    uint32_t max_virtserial_ports;
+    virtio_serial_conf serial;
     virtio_net_conf net;
     bool ioeventfd_disabled;
     bool ioeventfd_started;
@@ -770,12 +770,12 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
         proxy->class_code != PCI_CLASS_OTHERS)          /* qemu-kvm  */
         proxy->class_code = PCI_CLASS_COMMUNICATION_OTHER;
 
-    vdev = virtio_serial_init(&pci_dev->qdev, proxy->max_virtserial_ports);
+    vdev = virtio_serial_init(&pci_dev->qdev, &proxy->serial);
     if (!vdev) {
         return -1;
     }
     vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED
-                                        ? proxy->max_virtserial_ports + 1
+                                        ? proxy->serial.max_virtserial_ports + 1
                                         : proxy->nvectors;
     virtio_init_pci(proxy, vdev,
                     PCI_VENDOR_ID_REDHAT_QUMRANET,
@@ -902,8 +902,8 @@ static PCIDeviceInfo virtio_info[] = {
                                DEV_NVECTORS_UNSPECIFIED),
             DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
             DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
-            DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, max_virtserial_ports,
-                               31),
+            DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy,
+                               serial.max_virtserial_ports, 31),
             DEFINE_PROP_END_OF_LIST(),
         },
         .qdev.reset = virtio_pci_reset,
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 09e22aa..1753785 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -811,19 +811,19 @@ void virtio_serial_port_qdev_register(VirtIOSerialPortInfo *info)
     qdev_register(&info->qdev);
 }
 
-VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
+VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
 {
     VirtIOSerial *vser;
     VirtIODevice *vdev;
     uint32_t i, max_supported_ports;
 
-    if (!max_nr_ports)
+    if (!conf->max_virtserial_ports)
         return NULL;
 
     /* Each port takes 2 queues, and one pair is for the control queue */
     max_supported_ports = VIRTIO_PCI_QUEUE_MAX / 2 - 1;
 
-    if (max_nr_ports > max_supported_ports) {
+    if (conf->max_virtserial_ports > max_supported_ports) {
         error_report("maximum ports supported: %u", max_supported_ports);
         return NULL;
     }
@@ -839,9 +839,9 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
     vser->bus->vser = vser;
     QTAILQ_INIT(&vser->ports);
 
-    vser->bus->max_nr_ports = max_nr_ports;
-    vser->ivqs = qemu_malloc(max_nr_ports * sizeof(VirtQueue *));
-    vser->ovqs = qemu_malloc(max_nr_ports * sizeof(VirtQueue *));
+    vser->bus->max_nr_ports = conf->max_virtserial_ports;
+    vser->ivqs = qemu_malloc(conf->max_virtserial_ports * sizeof(VirtQueue *));
+    vser->ovqs = qemu_malloc(conf->max_virtserial_ports * sizeof(VirtQueue *));
 
     /* Add a queue for host to guest transfers for port 0 (backward compat) */
     vser->ivqs[0] = virtio_add_queue(vdev, 128, handle_input);
@@ -866,8 +866,8 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
         vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
     }
 
-    vser->config.max_nr_ports = max_nr_ports;
-    vser->ports_map = qemu_mallocz(((max_nr_ports + 31) / 32)
+    vser->config.max_nr_ports = conf->max_virtserial_ports;
+    vser->ports_map = qemu_mallocz(((conf->max_virtserial_ports + 31) / 32)
         * sizeof(vser->ports_map[0]));
     /*
      * Reserve location 0 for a console port for backward compat
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index a308196..de624c3 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -45,6 +45,11 @@ struct virtio_console_control {
     uint16_t value;		/* Extra information for the key */
 };
 
+struct virtio_serial_conf {
+    /* Max. number of ports we can have for a the virtio-serial device */
+    uint32_t max_virtserial_ports;
+};
+
 /* Some events for the internal messages (control packets) */
 #define VIRTIO_CONSOLE_DEVICE_READY	0
 #define VIRTIO_CONSOLE_PORT_ADD		1
diff --git a/hw/virtio.h b/hw/virtio.h
index 31d16e1..d0920a8 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -195,7 +195,8 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf);
 struct virtio_net_conf;
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
                               struct virtio_net_conf *net);
-VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports);
+typedef struct virtio_serial_conf virtio_serial_conf;
+VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *serial);
 VirtIODevice *virtio_balloon_init(DeviceState *dev);
 #ifdef CONFIG_LINUX
 VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);
-- 
1.7.3.5

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

* [Qemu-devel] [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control
  2011-02-03  6:17 [Qemu-devel] [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy Amit Shah
@ 2011-02-03  6:17 ` Amit Shah
  2011-02-07 15:47   ` [Qemu-devel] " Alex Williamson
  2011-02-07 15:46 ` [Qemu-devel] Re: [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy Alex Williamson
  1 sibling, 1 reply; 5+ messages in thread
From: Amit Shah @ 2011-02-03  6:17 UTC (permalink / raw)
  To: qemu list; +Cc: Amit Shah, Alex Williamson, Michael S. Tsirkin

Add a compat property for older machine types.  When this is used (via
-M pc-0.13, for example), the new flow control mechanisms will not be
used.  This is done to keep migration from a machine started with older
type on a pc-0.14+ qemu to an older machine working.

The property is named 'flow_control' and defaults to on.

Reported-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/pc_piix.c           |   16 ++++++++++
 hw/virtio-pci.c        |    2 +
 hw/virtio-serial-bus.c |   77 +++++++++++++++++++++++++++++++++++++++++-------
 hw/virtio-serial.h     |   12 +++++++
 4 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7b74473..05b0449 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -243,6 +243,10 @@ static QEMUMachine pc_machine_v0_13 = {
             .driver   = "PCI",
             .property = "command_serr_enable",
             .value    = "off",
+        },{
+            .driver   = "virtio-serial-pci",
+            .property = "flow_control",
+            .value    = stringify(0),
         },
         { /* end of list */ }
     },
@@ -263,6 +267,10 @@ static QEMUMachine pc_machine_v0_12 = {
             .property = "vectors",
             .value    = stringify(0),
         },{
+            .driver   = "virtio-serial-pci",
+            .property = "flow_control",
+            .value    = stringify(0),
+        },{
             .driver   = "VGA",
             .property = "rombar",
             .value    = stringify(0),
@@ -298,6 +306,10 @@ static QEMUMachine pc_machine_v0_11 = {
             .property = "vectors",
             .value    = stringify(0),
         },{
+            .driver   = "virtio-serial-pci",
+            .property = "flow_control",
+            .value    = stringify(0),
+        },{
             .driver   = "ide-drive",
             .property = "ver",
             .value    = "0.11",
@@ -341,6 +353,10 @@ static QEMUMachine pc_machine_v0_10 = {
             .property = "vectors",
             .value    = stringify(0),
         },{
+            .driver   = "virtio-serial-pci",
+            .property = "flow_control",
+            .value    = stringify(0),
+        },{
             .driver   = "virtio-net-pci",
             .property = "vectors",
             .value    = stringify(0),
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 952b5d2..530ce9d 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -904,6 +904,8 @@ static PCIDeviceInfo virtio_info[] = {
             DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
             DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy,
                                serial.max_virtserial_ports, 31),
+            DEFINE_PROP_UINT32("flow_control", VirtIOPCIProxy,
+                               serial.flow_control, 1),
             DEFINE_PROP_END_OF_LIST(),
         },
         .qdev.reset = virtio_pci_reset,
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 1753785..f681646 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -49,6 +49,8 @@ struct VirtIOSerial {
     uint32_t *ports_map;
 
     struct virtio_console_config config;
+
+    bool flow_control;
 };
 
 static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
@@ -123,12 +125,46 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
     virtio_notify(vdev, vq);
 }
 
+static void do_flush_queued_data_no_flow_control(VirtIOSerialPort *port,
+                                                 VirtQueue *vq,
+                                                 VirtIODevice *vdev)
+{
+    VirtQueueElement elem;
+
+    while (!port->throttled && virtqueue_pop(vq, &elem)) {
+        uint8_t *buf;
+        size_t ret, buf_size;
+
+        buf_size = iov_size(elem.out_sg, elem.out_num);
+        buf = qemu_malloc(buf_size);
+        ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
+
+        /*
+         * have_data has been modified to return the number of bytes
+         * successfully consumed.  We can't act upon that information
+         * in the no-flow-control implementation, so we'll discard it
+         * here.  No callers currently use flow control, but they
+         * should take this into account for backward compatibility.
+         */
+        port->info->have_data(port, buf, ret);
+        qemu_free(buf);
+
+        virtqueue_push(vq, &elem, 0);
+    }
+    virtio_notify(vdev, vq);
+}
+
 static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
                                  VirtIODevice *vdev)
 {
     assert(port);
     assert(virtio_queue_ready(vq));
 
+    if (!virtio_serial_flow_control_enabled(port)) {
+        do_flush_queued_data_no_flow_control(port, vq, vdev);
+        return;
+    }
+
     while (!port->throttled) {
         unsigned int i;
 
@@ -296,6 +332,15 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
     flush_queued_data(port);
 }
 
+bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port)
+{
+    if (!port) {
+        return false;
+    }
+
+    return port->vser->flow_control;
+}
+
 /* Guest wants to notify us of some event */
 static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
 {
@@ -533,17 +578,19 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
         qemu_put_byte(f, port->guest_connected);
         qemu_put_byte(f, port->host_connected);
 
-	elem_popped = 0;
-        if (port->elem.out_num) {
-            elem_popped = 1;
-        }
-        qemu_put_be32s(f, &elem_popped);
-        if (elem_popped) {
-            qemu_put_be32s(f, &port->iov_idx);
-            qemu_put_be64s(f, &port->iov_offset);
+        if (virtio_serial_flow_control_enabled(port)) {
+            elem_popped = 0;
+            if (port->elem.out_num) {
+                elem_popped = 1;
+            }
+            qemu_put_be32s(f, &elem_popped);
+            if (elem_popped) {
+                qemu_put_be32s(f, &port->iov_idx);
+                qemu_put_be64s(f, &port->iov_offset);
 
-            qemu_put_buffer(f, (unsigned char *)&port->elem,
-                            sizeof(port->elem));
+                qemu_put_buffer(f, (unsigned char *)&port->elem,
+                                sizeof(port->elem));
+            }
         }
     }
 }
@@ -816,6 +863,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
     VirtIOSerial *vser;
     VirtIODevice *vdev;
     uint32_t i, max_supported_ports;
+    unsigned int savevm_ver;
 
     if (!conf->max_virtserial_ports)
         return NULL;
@@ -881,11 +929,18 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
 
     vser->qdev = dev;
 
+    vser->flow_control = true;
+    savevm_ver = 3;
+    if (!conf->flow_control) {
+        vser->flow_control = false;
+        savevm_ver = 2;
+    }
+
     /*
      * Register for the savevm section with the virtio-console name
      * to preserve backward compat
      */
-    register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
+    register_savevm(dev, "virtio-console", -1, savevm_ver, virtio_serial_save,
                     virtio_serial_load, vser);
 
     return vdev;
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index de624c3..57d2dee 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -48,6 +48,13 @@ struct virtio_console_control {
 struct virtio_serial_conf {
     /* Max. number of ports we can have for a the virtio-serial device */
     uint32_t max_virtserial_ports;
+
+    /*
+     * Should this device behave the way it did in prior to pc-0.14
+     * (ie. no flow control)?  This will be necessary to allow
+     * migrations from a pre-0.14-machine type to older qemu code
+     */
+    uint32_t flow_control;
 };
 
 /* Some events for the internal messages (control packets) */
@@ -204,4 +211,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
  */
 void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
 
+/*
+ * Does this machine type disable the use of flow control?
+ */
+bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port);
+
 #endif
-- 
1.7.3.5

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

* [Qemu-devel] Re: [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy
  2011-02-03  6:17 [Qemu-devel] [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy Amit Shah
  2011-02-03  6:17 ` [Qemu-devel] [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control Amit Shah
@ 2011-02-07 15:46 ` Alex Williamson
  1 sibling, 0 replies; 5+ messages in thread
From: Alex Williamson @ 2011-02-07 15:46 UTC (permalink / raw)
  To: Amit Shah; +Cc: qemu list, Michael S. Tsirkin

On Thu, 2011-02-03 at 11:47 +0530, Amit Shah wrote:
> Instead of using a single variable to pass to the virtio_serial_init
> function, use a struct so that expanding the number of variables to be
> passed on later is easier.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/virtio-pci.c        |   12 ++++++------
>  hw/virtio-serial-bus.c |   16 ++++++++--------
>  hw/virtio-serial.h     |    5 +++++
>  hw/virtio.h            |    3 ++-
>  4 files changed, 21 insertions(+), 15 deletions(-)

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 3911b09..952b5d2 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -18,6 +18,7 @@
>  #include "virtio.h"
>  #include "virtio-blk.h"
>  #include "virtio-net.h"
> +#include "virtio-serial.h"
>  #include "pci.h"
>  #include "qemu-error.h"
>  #include "msix.h"
> @@ -109,8 +110,7 @@ typedef struct {
>  #ifdef CONFIG_LINUX
>      V9fsConf fsconf;
>  #endif
> -    /* Max. number of ports we can have for a the virtio-serial device */
> -    uint32_t max_virtserial_ports;
> +    virtio_serial_conf serial;
>      virtio_net_conf net;
>      bool ioeventfd_disabled;
>      bool ioeventfd_started;
> @@ -770,12 +770,12 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
>          proxy->class_code != PCI_CLASS_OTHERS)          /* qemu-kvm  */
>          proxy->class_code = PCI_CLASS_COMMUNICATION_OTHER;
>  
> -    vdev = virtio_serial_init(&pci_dev->qdev, proxy->max_virtserial_ports);
> +    vdev = virtio_serial_init(&pci_dev->qdev, &proxy->serial);
>      if (!vdev) {
>          return -1;
>      }
>      vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED
> -                                        ? proxy->max_virtserial_ports + 1
> +                                        ? proxy->serial.max_virtserial_ports + 1
>                                          : proxy->nvectors;
>      virtio_init_pci(proxy, vdev,
>                      PCI_VENDOR_ID_REDHAT_QUMRANET,
> @@ -902,8 +902,8 @@ static PCIDeviceInfo virtio_info[] = {
>                                 DEV_NVECTORS_UNSPECIFIED),
>              DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
>              DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
> -            DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, max_virtserial_ports,
> -                               31),
> +            DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy,
> +                               serial.max_virtserial_ports, 31),
>              DEFINE_PROP_END_OF_LIST(),
>          },
>          .qdev.reset = virtio_pci_reset,
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 09e22aa..1753785 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -811,19 +811,19 @@ void virtio_serial_port_qdev_register(VirtIOSerialPortInfo *info)
>      qdev_register(&info->qdev);
>  }
>  
> -VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
> +VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
>  {
>      VirtIOSerial *vser;
>      VirtIODevice *vdev;
>      uint32_t i, max_supported_ports;
>  
> -    if (!max_nr_ports)
> +    if (!conf->max_virtserial_ports)
>          return NULL;
>  
>      /* Each port takes 2 queues, and one pair is for the control queue */
>      max_supported_ports = VIRTIO_PCI_QUEUE_MAX / 2 - 1;
>  
> -    if (max_nr_ports > max_supported_ports) {
> +    if (conf->max_virtserial_ports > max_supported_ports) {
>          error_report("maximum ports supported: %u", max_supported_ports);
>          return NULL;
>      }
> @@ -839,9 +839,9 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
>      vser->bus->vser = vser;
>      QTAILQ_INIT(&vser->ports);
>  
> -    vser->bus->max_nr_ports = max_nr_ports;
> -    vser->ivqs = qemu_malloc(max_nr_ports * sizeof(VirtQueue *));
> -    vser->ovqs = qemu_malloc(max_nr_ports * sizeof(VirtQueue *));
> +    vser->bus->max_nr_ports = conf->max_virtserial_ports;
> +    vser->ivqs = qemu_malloc(conf->max_virtserial_ports * sizeof(VirtQueue *));
> +    vser->ovqs = qemu_malloc(conf->max_virtserial_ports * sizeof(VirtQueue *));
>  
>      /* Add a queue for host to guest transfers for port 0 (backward compat) */
>      vser->ivqs[0] = virtio_add_queue(vdev, 128, handle_input);
> @@ -866,8 +866,8 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
>          vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
>      }
>  
> -    vser->config.max_nr_ports = max_nr_ports;
> -    vser->ports_map = qemu_mallocz(((max_nr_ports + 31) / 32)
> +    vser->config.max_nr_ports = conf->max_virtserial_ports;
> +    vser->ports_map = qemu_mallocz(((conf->max_virtserial_ports + 31) / 32)
>          * sizeof(vser->ports_map[0]));
>      /*
>       * Reserve location 0 for a console port for backward compat
> diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
> index a308196..de624c3 100644
> --- a/hw/virtio-serial.h
> +++ b/hw/virtio-serial.h
> @@ -45,6 +45,11 @@ struct virtio_console_control {
>      uint16_t value;		/* Extra information for the key */
>  };
>  
> +struct virtio_serial_conf {
> +    /* Max. number of ports we can have for a the virtio-serial device */
> +    uint32_t max_virtserial_ports;
> +};
> +
>  /* Some events for the internal messages (control packets) */
>  #define VIRTIO_CONSOLE_DEVICE_READY	0
>  #define VIRTIO_CONSOLE_PORT_ADD		1
> diff --git a/hw/virtio.h b/hw/virtio.h
> index 31d16e1..d0920a8 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -195,7 +195,8 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf);
>  struct virtio_net_conf;
>  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
>                                struct virtio_net_conf *net);
> -VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports);
> +typedef struct virtio_serial_conf virtio_serial_conf;
> +VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *serial);
>  VirtIODevice *virtio_balloon_init(DeviceState *dev);
>  #ifdef CONFIG_LINUX
>  VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);

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

* [Qemu-devel] Re: [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control
  2011-02-03  6:17 ` [Qemu-devel] [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control Amit Shah
@ 2011-02-07 15:47   ` Alex Williamson
  2011-02-07 16:10     ` Michael S. Tsirkin
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Williamson @ 2011-02-07 15:47 UTC (permalink / raw)
  To: Amit Shah; +Cc: qemu list, Michael S. Tsirkin

On Thu, 2011-02-03 at 11:47 +0530, Amit Shah wrote:
> Add a compat property for older machine types.  When this is used (via
> -M pc-0.13, for example), the new flow control mechanisms will not be
> used.  This is done to keep migration from a machine started with older
> type on a pc-0.14+ qemu to an older machine working.
> 
> The property is named 'flow_control' and defaults to on.
> 
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/pc_piix.c           |   16 ++++++++++
>  hw/virtio-pci.c        |    2 +
>  hw/virtio-serial-bus.c |   77 +++++++++++++++++++++++++++++++++++++++++-------
>  hw/virtio-serial.h     |   12 +++++++
>  4 files changed, 96 insertions(+), 11 deletions(-)

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 7b74473..05b0449 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -243,6 +243,10 @@ static QEMUMachine pc_machine_v0_13 = {
>              .driver   = "PCI",
>              .property = "command_serr_enable",
>              .value    = "off",
> +        },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
>          },
>          { /* end of list */ }
>      },
> @@ -263,6 +267,10 @@ static QEMUMachine pc_machine_v0_12 = {
>              .property = "vectors",
>              .value    = stringify(0),
>          },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
> +        },{
>              .driver   = "VGA",
>              .property = "rombar",
>              .value    = stringify(0),
> @@ -298,6 +306,10 @@ static QEMUMachine pc_machine_v0_11 = {
>              .property = "vectors",
>              .value    = stringify(0),
>          },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
> +        },{
>              .driver   = "ide-drive",
>              .property = "ver",
>              .value    = "0.11",
> @@ -341,6 +353,10 @@ static QEMUMachine pc_machine_v0_10 = {
>              .property = "vectors",
>              .value    = stringify(0),
>          },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
> +        },{
>              .driver   = "virtio-net-pci",
>              .property = "vectors",
>              .value    = stringify(0),
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 952b5d2..530ce9d 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -904,6 +904,8 @@ static PCIDeviceInfo virtio_info[] = {
>              DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
>              DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy,
>                                 serial.max_virtserial_ports, 31),
> +            DEFINE_PROP_UINT32("flow_control", VirtIOPCIProxy,
> +                               serial.flow_control, 1),
>              DEFINE_PROP_END_OF_LIST(),
>          },
>          .qdev.reset = virtio_pci_reset,
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 1753785..f681646 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -49,6 +49,8 @@ struct VirtIOSerial {
>      uint32_t *ports_map;
>  
>      struct virtio_console_config config;
> +
> +    bool flow_control;
>  };
>  
>  static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
> @@ -123,12 +125,46 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
>      virtio_notify(vdev, vq);
>  }
>  
> +static void do_flush_queued_data_no_flow_control(VirtIOSerialPort *port,
> +                                                 VirtQueue *vq,
> +                                                 VirtIODevice *vdev)
> +{
> +    VirtQueueElement elem;
> +
> +    while (!port->throttled && virtqueue_pop(vq, &elem)) {
> +        uint8_t *buf;
> +        size_t ret, buf_size;
> +
> +        buf_size = iov_size(elem.out_sg, elem.out_num);
> +        buf = qemu_malloc(buf_size);
> +        ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
> +
> +        /*
> +         * have_data has been modified to return the number of bytes
> +         * successfully consumed.  We can't act upon that information
> +         * in the no-flow-control implementation, so we'll discard it
> +         * here.  No callers currently use flow control, but they
> +         * should take this into account for backward compatibility.
> +         */
> +        port->info->have_data(port, buf, ret);
> +        qemu_free(buf);
> +
> +        virtqueue_push(vq, &elem, 0);
> +    }
> +    virtio_notify(vdev, vq);
> +}
> +
>  static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
>                                   VirtIODevice *vdev)
>  {
>      assert(port);
>      assert(virtio_queue_ready(vq));
>  
> +    if (!virtio_serial_flow_control_enabled(port)) {
> +        do_flush_queued_data_no_flow_control(port, vq, vdev);
> +        return;
> +    }
> +
>      while (!port->throttled) {
>          unsigned int i;
>  
> @@ -296,6 +332,15 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
>      flush_queued_data(port);
>  }
>  
> +bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port)
> +{
> +    if (!port) {
> +        return false;
> +    }
> +
> +    return port->vser->flow_control;
> +}
> +
>  /* Guest wants to notify us of some event */
>  static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
>  {
> @@ -533,17 +578,19 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
>          qemu_put_byte(f, port->guest_connected);
>          qemu_put_byte(f, port->host_connected);
>  
> -	elem_popped = 0;
> -        if (port->elem.out_num) {
> -            elem_popped = 1;
> -        }
> -        qemu_put_be32s(f, &elem_popped);
> -        if (elem_popped) {
> -            qemu_put_be32s(f, &port->iov_idx);
> -            qemu_put_be64s(f, &port->iov_offset);
> +        if (virtio_serial_flow_control_enabled(port)) {
> +            elem_popped = 0;
> +            if (port->elem.out_num) {
> +                elem_popped = 1;
> +            }
> +            qemu_put_be32s(f, &elem_popped);
> +            if (elem_popped) {
> +                qemu_put_be32s(f, &port->iov_idx);
> +                qemu_put_be64s(f, &port->iov_offset);
>  
> -            qemu_put_buffer(f, (unsigned char *)&port->elem,
> -                            sizeof(port->elem));
> +                qemu_put_buffer(f, (unsigned char *)&port->elem,
> +                                sizeof(port->elem));
> +            }
>          }
>      }
>  }
> @@ -816,6 +863,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
>      VirtIOSerial *vser;
>      VirtIODevice *vdev;
>      uint32_t i, max_supported_ports;
> +    unsigned int savevm_ver;
>  
>      if (!conf->max_virtserial_ports)
>          return NULL;
> @@ -881,11 +929,18 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
>  
>      vser->qdev = dev;
>  
> +    vser->flow_control = true;
> +    savevm_ver = 3;
> +    if (!conf->flow_control) {
> +        vser->flow_control = false;
> +        savevm_ver = 2;
> +    }
> +
>      /*
>       * Register for the savevm section with the virtio-console name
>       * to preserve backward compat
>       */
> -    register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
> +    register_savevm(dev, "virtio-console", -1, savevm_ver, virtio_serial_save,
>                      virtio_serial_load, vser);
>  
>      return vdev;
> diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
> index de624c3..57d2dee 100644
> --- a/hw/virtio-serial.h
> +++ b/hw/virtio-serial.h
> @@ -48,6 +48,13 @@ struct virtio_console_control {
>  struct virtio_serial_conf {
>      /* Max. number of ports we can have for a the virtio-serial device */
>      uint32_t max_virtserial_ports;
> +
> +    /*
> +     * Should this device behave the way it did in prior to pc-0.14
> +     * (ie. no flow control)?  This will be necessary to allow
> +     * migrations from a pre-0.14-machine type to older qemu code
> +     */
> +    uint32_t flow_control;
>  };
>  
>  /* Some events for the internal messages (control packets) */
> @@ -204,4 +211,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
>   */
>  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
>  
> +/*
> + * Does this machine type disable the use of flow control?
> + */
> +bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port);
> +
>  #endif

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

* [Qemu-devel] Re: [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control
  2011-02-07 15:47   ` [Qemu-devel] " Alex Williamson
@ 2011-02-07 16:10     ` Michael S. Tsirkin
  0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2011-02-07 16:10 UTC (permalink / raw)
  To: Alex Williamson; +Cc: Amit Shah, qemu list

On Mon, Feb 07, 2011 at 08:47:11AM -0700, Alex Williamson wrote:
> On Thu, 2011-02-03 at 11:47 +0530, Amit Shah wrote:
> > Add a compat property for older machine types.  When this is used (via
> > -M pc-0.13, for example), the new flow control mechanisms will not be
> > used.  This is done to keep migration from a machine started with older
> > type on a pc-0.14+ qemu to an older machine working.
> > 
> > The property is named 'flow_control' and defaults to on.
> > 
> > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  hw/pc_piix.c           |   16 ++++++++++
> >  hw/virtio-pci.c        |    2 +
> >  hw/virtio-serial-bus.c |   77 +++++++++++++++++++++++++++++++++++++++++-------
> >  hw/virtio-serial.h     |   12 +++++++
> >  4 files changed, 96 insertions(+), 11 deletions(-)
> 
> Acked-by: Alex Williamson <alex.williamson@redhat.com>

Before merging this, I think we need to have a discussion on
cross version migration. Started a separate thread for that.

> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index 7b74473..05b0449 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -243,6 +243,10 @@ static QEMUMachine pc_machine_v0_13 = {
> >              .driver   = "PCI",
> >              .property = "command_serr_enable",
> >              .value    = "off",
> > +        },{
> > +            .driver   = "virtio-serial-pci",
> > +            .property = "flow_control",
> > +            .value    = stringify(0),
> >          },
> >          { /* end of list */ }
> >      },
> > @@ -263,6 +267,10 @@ static QEMUMachine pc_machine_v0_12 = {
> >              .property = "vectors",
> >              .value    = stringify(0),
> >          },{
> > +            .driver   = "virtio-serial-pci",
> > +            .property = "flow_control",
> > +            .value    = stringify(0),
> > +        },{
> >              .driver   = "VGA",
> >              .property = "rombar",
> >              .value    = stringify(0),
> > @@ -298,6 +306,10 @@ static QEMUMachine pc_machine_v0_11 = {
> >              .property = "vectors",
> >              .value    = stringify(0),
> >          },{
> > +            .driver   = "virtio-serial-pci",
> > +            .property = "flow_control",
> > +            .value    = stringify(0),
> > +        },{
> >              .driver   = "ide-drive",
> >              .property = "ver",
> >              .value    = "0.11",
> > @@ -341,6 +353,10 @@ static QEMUMachine pc_machine_v0_10 = {
> >              .property = "vectors",
> >              .value    = stringify(0),
> >          },{
> > +            .driver   = "virtio-serial-pci",
> > +            .property = "flow_control",
> > +            .value    = stringify(0),
> > +        },{
> >              .driver   = "virtio-net-pci",
> >              .property = "vectors",
> >              .value    = stringify(0),
> > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> > index 952b5d2..530ce9d 100644
> > --- a/hw/virtio-pci.c
> > +++ b/hw/virtio-pci.c
> > @@ -904,6 +904,8 @@ static PCIDeviceInfo virtio_info[] = {
> >              DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
> >              DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy,
> >                                 serial.max_virtserial_ports, 31),
> > +            DEFINE_PROP_UINT32("flow_control", VirtIOPCIProxy,
> > +                               serial.flow_control, 1),
> >              DEFINE_PROP_END_OF_LIST(),
> >          },
> >          .qdev.reset = virtio_pci_reset,
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index 1753785..f681646 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -49,6 +49,8 @@ struct VirtIOSerial {
> >      uint32_t *ports_map;
> >  
> >      struct virtio_console_config config;
> > +
> > +    bool flow_control;
> >  };
> >  
> >  static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
> > @@ -123,12 +125,46 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
> >      virtio_notify(vdev, vq);
> >  }
> >  
> > +static void do_flush_queued_data_no_flow_control(VirtIOSerialPort *port,
> > +                                                 VirtQueue *vq,
> > +                                                 VirtIODevice *vdev)
> > +{
> > +    VirtQueueElement elem;
> > +
> > +    while (!port->throttled && virtqueue_pop(vq, &elem)) {
> > +        uint8_t *buf;
> > +        size_t ret, buf_size;
> > +
> > +        buf_size = iov_size(elem.out_sg, elem.out_num);
> > +        buf = qemu_malloc(buf_size);
> > +        ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
> > +
> > +        /*
> > +         * have_data has been modified to return the number of bytes
> > +         * successfully consumed.  We can't act upon that information
> > +         * in the no-flow-control implementation, so we'll discard it
> > +         * here.  No callers currently use flow control, but they
> > +         * should take this into account for backward compatibility.
> > +         */
> > +        port->info->have_data(port, buf, ret);
> > +        qemu_free(buf);
> > +
> > +        virtqueue_push(vq, &elem, 0);
> > +    }
> > +    virtio_notify(vdev, vq);
> > +}
> > +
> >  static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
> >                                   VirtIODevice *vdev)
> >  {
> >      assert(port);
> >      assert(virtio_queue_ready(vq));
> >  
> > +    if (!virtio_serial_flow_control_enabled(port)) {
> > +        do_flush_queued_data_no_flow_control(port, vq, vdev);
> > +        return;
> > +    }
> > +
> >      while (!port->throttled) {
> >          unsigned int i;
> >  
> > @@ -296,6 +332,15 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
> >      flush_queued_data(port);
> >  }
> >  
> > +bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port)
> > +{
> > +    if (!port) {
> > +        return false;
> > +    }
> > +
> > +    return port->vser->flow_control;
> > +}
> > +
> >  /* Guest wants to notify us of some event */
> >  static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
> >  {
> > @@ -533,17 +578,19 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
> >          qemu_put_byte(f, port->guest_connected);
> >          qemu_put_byte(f, port->host_connected);
> >  
> > -	elem_popped = 0;
> > -        if (port->elem.out_num) {
> > -            elem_popped = 1;
> > -        }
> > -        qemu_put_be32s(f, &elem_popped);
> > -        if (elem_popped) {
> > -            qemu_put_be32s(f, &port->iov_idx);
> > -            qemu_put_be64s(f, &port->iov_offset);
> > +        if (virtio_serial_flow_control_enabled(port)) {
> > +            elem_popped = 0;
> > +            if (port->elem.out_num) {
> > +                elem_popped = 1;
> > +            }
> > +            qemu_put_be32s(f, &elem_popped);
> > +            if (elem_popped) {
> > +                qemu_put_be32s(f, &port->iov_idx);
> > +                qemu_put_be64s(f, &port->iov_offset);
> >  
> > -            qemu_put_buffer(f, (unsigned char *)&port->elem,
> > -                            sizeof(port->elem));
> > +                qemu_put_buffer(f, (unsigned char *)&port->elem,
> > +                                sizeof(port->elem));
> > +            }
> >          }
> >      }
> >  }
> > @@ -816,6 +863,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
> >      VirtIOSerial *vser;
> >      VirtIODevice *vdev;
> >      uint32_t i, max_supported_ports;
> > +    unsigned int savevm_ver;
> >  
> >      if (!conf->max_virtserial_ports)
> >          return NULL;
> > @@ -881,11 +929,18 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
> >  
> >      vser->qdev = dev;
> >  
> > +    vser->flow_control = true;
> > +    savevm_ver = 3;
> > +    if (!conf->flow_control) {
> > +        vser->flow_control = false;
> > +        savevm_ver = 2;
> > +    }
> > +
> >      /*
> >       * Register for the savevm section with the virtio-console name
> >       * to preserve backward compat
> >       */
> > -    register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
> > +    register_savevm(dev, "virtio-console", -1, savevm_ver, virtio_serial_save,
> >                      virtio_serial_load, vser);
> >  
> >      return vdev;
> > diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
> > index de624c3..57d2dee 100644
> > --- a/hw/virtio-serial.h
> > +++ b/hw/virtio-serial.h
> > @@ -48,6 +48,13 @@ struct virtio_console_control {
> >  struct virtio_serial_conf {
> >      /* Max. number of ports we can have for a the virtio-serial device */
> >      uint32_t max_virtserial_ports;
> > +
> > +    /*
> > +     * Should this device behave the way it did in prior to pc-0.14
> > +     * (ie. no flow control)?  This will be necessary to allow
> > +     * migrations from a pre-0.14-machine type to older qemu code
> > +     */
> > +    uint32_t flow_control;
> >  };
> >  
> >  /* Some events for the internal messages (control packets) */
> > @@ -204,4 +211,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
> >   */
> >  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
> >  
> > +/*
> > + * Does this machine type disable the use of flow control?
> > + */
> > +bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port);
> > +
> >  #endif
> 
> 

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

end of thread, other threads:[~2011-02-07 16:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-03  6:17 [Qemu-devel] [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy Amit Shah
2011-02-03  6:17 ` [Qemu-devel] [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control Amit Shah
2011-02-07 15:47   ` [Qemu-devel] " Alex Williamson
2011-02-07 16:10     ` Michael S. Tsirkin
2011-02-07 15:46 ` [Qemu-devel] Re: [PATCH master/0.14 1/2] virtio-serial: Use a struct to pass config information from proxy Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).