Re: [PATCH v2 5/6] virtio-serial-bus: add terminal resize messages

[Michael S. Tsirkin]

On Wed, Jun 24, 2020 at 01:26:39PM +0200, Szymon Lukasz wrote:
> Implement the part of the virtio spec that allows to notify the virtio
> driver about terminal resizes. The virtio spec contains two methods to
> achieve that:
> 
> For legacy drivers, we have only one port and we put the terminal size
> in the config space and inject the config changed interrupt.
> 
> For multiport devices, we use the control virtqueue to send a packet
> containing the terminal size. Note that the Linux kernel expects
> the fields indicating the number of rows and columns in a packet to be
> in a different order than the one specified in the current version of
> the virtio spec. We follow the Linux implementation, so hopefully there
> is no implementation of this functionality conforming to the spec.
> 
> Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>

Looks right to me:


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/char/trace-events              |  1 +
>  hw/char/virtio-serial-bus.c       | 42 +++++++++++++++++++++++++++++--
>  hw/core/machine.c                 |  1 +
>  include/hw/virtio/virtio-serial.h |  5 ++++
>  4 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/char/trace-events b/hw/char/trace-events
> index d20eafd56f..be40df47ea 100644
> --- a/hw/char/trace-events
> +++ b/hw/char/trace-events
> @@ -10,6 +10,7 @@ serial_ioport_write(uint16_t addr, uint8_t value) "write 
> addr 0x%02x val 0x%02x"
>  
>  # virtio-serial-bus.c
>  virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t 
> value) "port %u, event %u, value %u"
> +virtio_serial_send_console_resize(unsigned int port, uint16_t cols, uint16_t 
> rows) "port %u, cols %u, rows %u"
>  virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u, 
> throttle %d"
>  virtio_serial_handle_control_message(uint16_t event, uint16_t value) "event 
> %u, value %u"
>  virtio_serial_handle_control_message_port(unsigned int port) "port %u"
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 262089c0c9..6d9e94a64e 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -261,6 +261,42 @@ static size_t send_control_event(VirtIOSerial *vser, 
> uint32_t port_id,
>      return send_control_msg(vser, &cpkt, sizeof(cpkt));
>  }
>  
> +/*
> + * This struct should be added to the Linux kernel uapi headers
> + * and later imported to standard-headers/linux/virtio_console.h
> + */
> +struct virtio_console_resize {
> +    __virtio16 rows;
> +    __virtio16 cols;
> +};
> +
> +void virtio_serial_send_console_resize(VirtIOSerialPort *port,
> +                                       uint16_t cols, uint16_t rows)
> +{
> +    VirtIOSerial *vser = port->vser;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(vser);
> +
> +    if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
> +        struct {
> +            struct virtio_console_control control;
> +            struct virtio_console_resize resize;
> +        } buffer;
> +
> +        virtio_stl_p(vdev, &buffer.control.id, port->id);
> +        virtio_stw_p(vdev, &buffer.control.event, VIRTIO_CONSOLE_RESIZE);
> +        virtio_stw_p(vdev, &buffer.resize.cols, cols);
> +        virtio_stw_p(vdev, &buffer.resize.rows, rows);
> +
> +        trace_virtio_serial_send_console_resize(port->id, cols, rows);
> +        send_control_msg(vser, &buffer, sizeof(buffer));
> +
> +    } else if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) {
> +        vser->port0_cols = cols;
> +        vser->port0_rows = rows;
> +        virtio_notify_config(vdev);
> +    }
> +}
> +
>  /* Functions for use inside qemu to open and read from/write to ports */
>  int virtio_serial_open(VirtIOSerialPort *port)
>  {
> @@ -572,8 +608,8 @@ static void get_config(VirtIODevice *vdev, uint8_t 
> *config_data)
>      struct virtio_console_config *config =
>          (struct virtio_console_config *)config_data;
>  
> -    config->cols = 0;
> -    config->rows = 0;
> +    config->cols = virtio_tswap16(vdev, vser->port0_cols);
> +    config->rows = virtio_tswap16(vdev, vser->port0_rows);
>      config->max_nr_ports = virtio_tswap32(vdev,
>                                            vser->serial.max_virtserial_ports);
>  }
> @@ -1168,6 +1204,8 @@ static Property virtio_serial_properties[] = {
>                                                    31),
>      DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
>                        VIRTIO_CONSOLE_F_EMERG_WRITE, true),
> +    DEFINE_PROP_BIT64("console-size", VirtIOSerial, host_features,
> +                      VIRTIO_CONSOLE_F_SIZE, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 1d80ab0e1d..c370c220f0 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -30,6 +30,7 @@
>  
>  GlobalProperty hw_compat_5_0[] = {
>      { "virtio-balloon-device", "page-poison", "false" },
> +    { "virtio-serial-device", "console-size", "off" },
>  };
>  const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
>  
> diff --git a/include/hw/virtio/virtio-serial.h 
> b/include/hw/virtio/virtio-serial.h
> index ed3e916b68..1d6436c0b1 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -188,6 +188,8 @@ struct VirtIOSerial {
>      virtio_serial_conf serial;
>  
>      uint64_t host_features;
> +
> +    uint16_t port0_cols, port0_rows;
>  };
>  
>  /* Interface to the virtio-serial bus */
> @@ -222,6 +224,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
>   */
>  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
>  
> +void virtio_serial_send_console_resize(VirtIOSerialPort *port,
> +                                       uint16_t cols, uint16_t rows);
> +
>  #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
>  #define VIRTIO_SERIAL(obj) \
>          OBJECT_CHECK(VirtIOSerial, (obj), TYPE_VIRTIO_SERIAL)
> -- 
> 2.27.0