Re: [PATCH v3 04/11] virtio: console: Add freeze and restore handlers to support S4

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Nov 17, 2011 at 05:27:35PM +0530, Amit Shah wrote:
> Remove all vqs and associated buffers in the freeze callback which
> prepares us to go into hibernation state.  On restore, re-create all the
> vqs and populate the input vqs with buffers to get to the pre-hibernate
> state.
> 
> Note: Any outstanding unconsumed buffers are discarded; which means
> there's a possibility of data loss in case the host or the guest didn't
> consume any data already present in the vqs.  This can be addressed in a
> later patch series, perhaps in virtio common code.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  drivers/char/virtio_console.c |   58 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index e14f5aa..fd2fd6f 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1844,6 +1844,60 @@ static unsigned int features[] = {
>  	VIRTIO_CONSOLE_F_MULTIPORT,
>  };
>  
> +#ifdef CONFIG_PM
> +static int virtcons_freeze(struct virtio_device *vdev)
> +{
> +	struct ports_device *portdev;
> +	struct port *port;
> +
> +	portdev = vdev->priv;
> +
> +	vdev->config->reset(vdev);

This does a reset but that's not a guarantee that
interrupt is not running on another CPU.

> +
> +	cancel_work_sync(&portdev->control_work);

And then work can get scheduled after this point.

> +	remove_controlq_data(portdev);

And after this point this will lead to a use after free.

> +
> +	list_for_each_entry(port, &portdev->ports, list) {
> +		/*
> +		 * We'll ask the host later if the new invocation has
> +		 * the port opened or closed.
> +		 */
> +		port->host_connected = false;
> +		remove_port_data(port);
> +	}
> +	remove_vqs(portdev);
> +
> +	return 0;
> +}
> +
> +static int virtcons_restore(struct virtio_device *vdev)
> +{
> +	struct ports_device *portdev;
> +	struct port *port;
> +	int ret;
> +
> +	portdev = vdev->priv;
> +
> +	ret = init_vqs(portdev);
> +	if (ret)
> +		return ret;
> +
> +	if (use_multiport(portdev))
> +		fill_queue(portdev->c_ivq, &portdev->cvq_lock);
> +
> +	list_for_each_entry(port, &portdev->ports, list) {
> +		port->in_vq = portdev->in_vqs[port->id];
> +		port->out_vq = portdev->out_vqs[port->id];
> +
> +		fill_queue(port->in_vq, &port->inbuf_lock);
> +
> +		/* Get port open/close status on the host */
> +		send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
> +	}
> +	return 0;
> +}
> +#endif
> +
>  static struct virtio_driver virtio_console = {
>  	.feature_table = features,
>  	.feature_table_size = ARRAY_SIZE(features),
> @@ -1853,6 +1907,10 @@ static struct virtio_driver virtio_console = {
>  	.probe =	virtcons_probe,
>  	.remove =	virtcons_remove,
>  	.config_changed = config_intr,
> +#ifdef CONFIG_PM
> +	.freeze =	virtcons_freeze,
> +	.restore =	virtcons_restore,
> +#endif
>  };
>  
>  static int __init init(void)
> -- 
> 1.7.7.1