From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50288 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmTJD-0001xr-Bv for qemu-devel@nongnu.org; Mon, 07 Feb 2011 10:47:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmTJ4-0001OE-Eh for qemu-devel@nongnu.org; Mon, 07 Feb 2011 10:47:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64309) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmTJ4-0001O0-6x for qemu-devel@nongnu.org; Mon, 07 Feb 2011 10:47:14 -0500 From: Alex Williamson In-Reply-To: <349e93a4cfc6e1effc1b681cae53f805fdb9624e.1296713825.git.amit.shah@redhat.com> References: <4d9a296e9a64abe6743655a4778ee7f6f2efc719.1296713825.git.amit.shah@redhat.com> <349e93a4cfc6e1effc1b681cae53f805fdb9624e.1296713825.git.amit.shah@redhat.com> Content-Type: text/plain; charset="UTF-8" Date: Mon, 07 Feb 2011 08:47:11 -0700 Message-ID: <1297093631.3518.7.camel@x201> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 > Signed-off-by: Amit Shah > --- > 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 > 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