All of lore.kernel.org
 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 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.