[Qemu-devel] [PATCH] virtio-serial: virtio console emergency write support

[Qemu-devel] [PATCH] virtio-serial: virtio console emergency write support

[Sascha Silbe]

Add support for the virtio 1.0 "emergency write"
(VIRTIO_CONSOLE_F_EMERG_WRITE) feature. This is useful for early guest
debugging and might be used in cases where the guest crashes so badly
that it cannot use virtqueues.

Disabled for compatibility machines to avoid exposing a new feature to
existing guests.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 hw/char/virtio-serial-bus.c       | 49 ++++++++++++++++++++++++++++++++++++---
 include/hw/compat.h               |  6 ++++-
 include/hw/virtio/virtio-serial.h |  2 ++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index db57a38..db2a9f1 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -75,6 +75,19 @@ static VirtIOSerialPort *find_port_by_name(char *name)
     return NULL;
 }
 
+static VirtIOSerialPort *find_first_connected_console(VirtIOSerial *vser)
+{
+    VirtIOSerialPort *port;
+
+    QTAILQ_FOREACH(port, &vser->ports, next) {
+        VirtIOSerialPortClass const *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+        if (vsc->is_console && port->host_connected) {
+            return port;
+        }
+    }
+    return NULL;
+}
+
 static bool use_multiport(VirtIOSerial *vser)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vser);
@@ -528,6 +541,7 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
 
     vser = VIRTIO_SERIAL(vdev);
 
+    features |= vser->host_features;
     if (vser->bus.max_nr_ports > 1) {
         virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT);
     }
@@ -547,6 +561,29 @@ static void get_config(VirtIODevice *vdev, uint8_t *config_data)
                                           vser->serial.max_virtserial_ports);
 }
 
+/* Guest sent new config info */
+static void set_config(VirtIODevice *vdev, const uint8_t *config_data)
+{
+    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
+    struct virtio_console_config *config =
+        (struct virtio_console_config *)config_data;
+    uint8_t emerg_wr_lo = le32_to_cpu(config->emerg_wr);
+    VirtIOSerialPort *port = find_first_connected_console(vser);
+    VirtIOSerialPortClass *vsc;
+
+    if (!config->emerg_wr) {
+        return;
+    }
+    /* Make sure we don't misdetect an emergency write when the guest
+     * does a short config write after an emergency write. */
+    config->emerg_wr = 0;
+    if (!port) {
+        return;
+    }
+    vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+    (void)vsc->have_data(port, &emerg_wr_lo, 1);
+}
+
 static void guest_reset(VirtIOSerial *vser)
 {
     VirtIOSerialPort *port;
@@ -967,6 +1004,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSerial *vser = VIRTIO_SERIAL(dev);
     uint32_t i, max_supported_ports;
+    size_t config_size = sizeof(struct virtio_console_config);
 
     if (!vser->serial.max_virtserial_ports) {
         error_setg(errp, "Maximum number of serial ports not specified");
@@ -981,10 +1019,12 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    /* We don't support emergency write, skip it for now. */
-    /* TODO: cleaner fix, depending on host features. */
+    if (!virtio_has_feature(vser->host_features,
+                            VIRTIO_CONSOLE_F_EMERG_WRITE)) {
+        config_size = offsetof(struct virtio_console_config, emerg_wr);
+    }
     virtio_init(vdev, "virtio-serial", VIRTIO_ID_CONSOLE,
-                offsetof(struct virtio_console_config, emerg_wr));
+                config_size);
 
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
     qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
@@ -1080,6 +1120,8 @@ VMSTATE_VIRTIO_DEVICE(console, 3, virtio_serial_load, virtio_vmstate_save);
 static Property virtio_serial_properties[] = {
     DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,
                                                   31),
+    DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
+                      VIRTIO_CONSOLE_F_EMERG_WRITE, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1098,6 +1140,7 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
     vdc->unrealize = virtio_serial_device_unrealize;
     vdc->get_features = get_features;
     vdc->get_config = get_config;
+    vdc->set_config = set_config;
     vdc->set_status = set_status;
     vdc->reset = vser_reset;
     vdc->save = virtio_serial_save_device;
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 08dd4fb..d48507a 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,11 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_2_7 \
-        /* empty */
+    {\
+        .driver   = "virtio-serial-device",\
+        .property = "emergency-write",\
+        .value    = "off",\
+    },
 
 #define HW_COMPAT_2_6 \
     {\
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 730c88d..b19c447 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -184,6 +184,8 @@ struct VirtIOSerial {
     struct VirtIOSerialPostLoad *post_load;
 
     virtio_serial_conf serial;
+
+    uint64_t host_features;
 };
 
 /* Interface to the virtio-serial bus */
-- 
1.9.1

Re: [Qemu-devel] [PATCH] virtio-serial: virtio console emergency write support

[Amit Shah]

Hi,

On (Wed) 21 Sep 2016 [14:17:48], Sascha Silbe wrote:
> Add support for the virtio 1.0 "emergency write"
> (VIRTIO_CONSOLE_F_EMERG_WRITE) feature. This is useful for early guest
> debugging and might be used in cases where the guest crashes so badly
> that it cannot use virtqueues.
> 
> Disabled for compatibility machines to avoid exposing a new feature to
> existing guests.
> 
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>

This looks fine, but can you split the patch - adding
find_first_connected_console(), set_config, and then finally enabling
the emergency_write feature?

Thanks,


		Amit

Re: [Qemu-devel] [PATCH] virtio-serial: virtio console emergency write support

[Sascha Silbe]

Dear Amit,

Amit Shah <amit.shah@redhat.com> writes:

> On (Wed) 21 Sep 2016 [14:17:48], Sascha Silbe wrote:
>> Add support for the virtio 1.0 "emergency write"
>> (VIRTIO_CONSOLE_F_EMERG_WRITE) feature. This is useful for early guest
>> debugging and might be used in cases where the guest crashes so badly
>> that it cannot use virtqueues.
>> 
>> Disabled for compatibility machines to avoid exposing a new feature to
>> existing guests.
>> 
>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
>
> This looks fine, but can you split the patch - adding
> find_first_connected_console(), set_config, and then finally enabling
> the emergency_write feature?

Sure. I've split it into two patches: One introducing
find_first_connected_console() + set_config() but with no way for the
guest to actually exploit it. The other patch exposing the emergency
write virtio feature to the guest and disabling it for older machines
using the compat property.

Will send the updated series once the build tests complete.

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641