From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34646 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmTgF-0003qX-MI for qemu-devel@nongnu.org; Mon, 07 Feb 2011 11:11:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmTgE-0006ng-7f for qemu-devel@nongnu.org; Mon, 07 Feb 2011 11:11:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37901) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmTgD-0006nU-UU for qemu-devel@nongnu.org; Mon, 07 Feb 2011 11:11:10 -0500 Date: Mon, 7 Feb 2011 18:10:57 +0200 From: "Michael S. Tsirkin" Message-ID: <20110207161057.GF25106@redhat.com> References: <4d9a296e9a64abe6743655a4778ee7f6f2efc719.1296713825.git.amit.shah@redhat.com> <349e93a4cfc6e1effc1b681cae53f805fdb9624e.1296713825.git.amit.shah@redhat.com> <1297093631.3518.7.camel@x201> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1297093631.3518.7.camel@x201> 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: 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 > > 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 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 > >