Re: [PATCH v8 17/50] KVM: s390: Set virtio-ccw transport revision

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

On Mon, Dec 01, 2014 at 06:04:11PM +0200, Michael S. Tsirkin wrote:
> From: Thomas Huth <thuth@linux.vnet.ibm.com>
> 
> With the new SET-VIRTIO-REVISION command of the virtio 1.0 standard, we
> can now negotiate the virtio-ccw revision after setting a channel online.
> 
> Note that we don't negotiate version 1 yet.
> 
> [Cornelia Huck: reworked revision loop a bit]
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Looking at this some more: to me it looks
like this command should be repeated whenever we reset
the device.

No?


> ---
>  drivers/s390/kvm/virtio_ccw.c | 63 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index 65d0c80..f6eb47b 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -55,6 +55,7 @@ struct virtio_ccw_device {
>  	struct ccw_device *cdev;
>  	__u32 curr_io;
>  	int err;
> +	unsigned int revision; /* Transport revision */
>  	wait_queue_head_t wait_q;
>  	spinlock_t lock;
>  	struct list_head virtqueues;
> @@ -86,6 +87,15 @@ struct virtio_thinint_area {
>  	u8 isc;
>  } __packed;
>  
> +struct virtio_rev_info {
> +	__u16 revision;
> +	__u16 length;
> +	__u8 data[];
> +};
> +
> +/* the highest virtio-ccw revision we support */
> +#define VIRTIO_CCW_REV_MAX 0
> +
>  struct virtio_ccw_vq_info {
>  	struct virtqueue *vq;
>  	int num;
> @@ -122,6 +132,7 @@ static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
>  #define CCW_CMD_WRITE_STATUS 0x31
>  #define CCW_CMD_READ_VQ_CONF 0x32
>  #define CCW_CMD_SET_IND_ADAPTER 0x73
> +#define CCW_CMD_SET_VIRTIO_REV 0x83
>  
>  #define VIRTIO_CCW_DOING_SET_VQ 0x00010000
>  #define VIRTIO_CCW_DOING_RESET 0x00040000
> @@ -134,6 +145,7 @@ static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
>  #define VIRTIO_CCW_DOING_READ_VQ_CONF 0x02000000
>  #define VIRTIO_CCW_DOING_SET_CONF_IND 0x04000000
>  #define VIRTIO_CCW_DOING_SET_IND_ADAPTER 0x08000000
> +#define VIRTIO_CCW_DOING_SET_VIRTIO_REV 0x10000000
>  #define VIRTIO_CCW_INTPARM_MASK 0xffff0000
>  
>  static struct virtio_ccw_device *to_vc_device(struct virtio_device *vdev)
> @@ -933,6 +945,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
>  		case VIRTIO_CCW_DOING_RESET:
>  		case VIRTIO_CCW_DOING_READ_VQ_CONF:
>  		case VIRTIO_CCW_DOING_SET_IND_ADAPTER:
> +		case VIRTIO_CCW_DOING_SET_VIRTIO_REV:
>  			vcdev->curr_io &= ~activity;
>  			wake_up(&vcdev->wait_q);
>  			break;
> @@ -1048,6 +1061,51 @@ static int virtio_ccw_offline(struct ccw_device *cdev)
>  	return 0;
>  }
>  
> +static int virtio_ccw_set_transport_rev(struct virtio_ccw_device *vcdev)
> +{
> +	struct virtio_rev_info *rev;
> +	struct ccw1 *ccw;
> +	int ret;
> +
> +	ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
> +	if (!ccw)
> +		return -ENOMEM;
> +	rev = kzalloc(sizeof(*rev), GFP_DMA | GFP_KERNEL);
> +	if (!rev) {
> +		kfree(ccw);
> +		return -ENOMEM;
> +	}
> +
> +	/* Set transport revision */
> +	ccw->cmd_code = CCW_CMD_SET_VIRTIO_REV;
> +	ccw->flags = 0;
> +	ccw->count = sizeof(*rev);
> +	ccw->cda = (__u32)(unsigned long)rev;
> +
> +	vcdev->revision = VIRTIO_CCW_REV_MAX;
> +	do {
> +		rev->revision = vcdev->revision;
> +		/* none of our supported revisions carry payload */
> +		rev->length = 0;
> +		ret = ccw_io_helper(vcdev, ccw,
> +				    VIRTIO_CCW_DOING_SET_VIRTIO_REV);
> +		if (ret == -EOPNOTSUPP) {
> +			if (vcdev->revision == 0)
> +				/*
> +				 * The host device does not support setting
> +				 * the revision: let's operate it in legacy
> +				 * mode.
> +				 */
> +				ret = 0;
> +			else
> +				vcdev->revision--;
> +		}
> +	} while (ret == -EOPNOTSUPP);
> +
> +	kfree(ccw);
> +	kfree(rev);
> +	return ret;
> +}
>  
>  static int virtio_ccw_online(struct ccw_device *cdev)
>  {
> @@ -1088,6 +1146,11 @@ static int virtio_ccw_online(struct ccw_device *cdev)
>  	spin_unlock_irqrestore(get_ccwdev_lock(cdev), flags);
>  	vcdev->vdev.id.vendor = cdev->id.cu_type;
>  	vcdev->vdev.id.device = cdev->id.cu_model;
> +
> +	ret = virtio_ccw_set_transport_rev(vcdev);
> +	if (ret)
> +		goto out_free;
> +
>  	ret = register_virtio_device(&vcdev->vdev);
>  	if (ret) {
>  		dev_warn(&cdev->dev, "Failed to register virtio device: %d\n",
> -- 
> MST
>