[PATCH-v2] tcm_vhost: Avoid VIRTIO_RING_F_EVENT_IDX feature bit

[PATCH-v2] tcm_vhost: Avoid VIRTIO_RING_F_EVENT_IDX feature bit

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds a VHOST_SCSI_FEATURES mask minus VIRTIO_RING_F_EVENT_IDX
so that vhost-scsi-pci userspace will strip this feature bit once
GET_FEATURES reports it as being unsupported on the host.

This is to avoid a bug where ->handle_kicks() are missed when EVENT_IDX
is enabled by default in userspace code.

(mst: Rename to VHOST_SCSI_FEATURES add comment)

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Asias He <asias@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/vhost/tcm_vhost.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 0524267..4a68c1f 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -60,6 +60,15 @@ enum {
 	VHOST_SCSI_VQ_IO = 2,
 };
 
+/*
+ * VIRTIO_RING_F_EVENT_IDX seems broken. Not sure the bug is in
+ * kernel but disabling it helps.
+ * TODO: debug and remove the workaround.
+ */
+enum {
+	VHOST_SCSI_FEATURES = VHOST_FEATURES & (~VIRTIO_RING_F_EVENT_IDX)
+};
+
 #define VHOST_SCSI_MAX_TARGET	256
 #define VHOST_SCSI_MAX_VQ	128
 
@@ -981,7 +990,7 @@ static void vhost_scsi_flush(struct vhost_scsi *vs)
 
 static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
 {
-	if (features & ~VHOST_FEATURES)
+	if (features & ~VHOST_SCSI_FEATURES)
 		return -EOPNOTSUPP;
 
 	mutex_lock(&vs->dev.mutex);
@@ -1027,7 +1036,7 @@ static long vhost_scsi_ioctl(struct file *f, unsigned int ioctl,
 			return -EFAULT;
 		return 0;
 	case VHOST_GET_FEATURES:
-		features = VHOST_FEATURES;
+		features = VHOST_SCSI_FEATURES;
 		if (copy_to_user(featurep, &features, sizeof features))
 			return -EFAULT;
 		return 0;
-- 
1.7.2.5

Re: [PATCH-v2] tcm_vhost: Avoid VIRTIO_RING_F_EVENT_IDX feature bit

[Asias He]

On Fri, Mar 29, 2013 at 12:50:26AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch adds a VHOST_SCSI_FEATURES mask minus VIRTIO_RING_F_EVENT_IDX
> so that vhost-scsi-pci userspace will strip this feature bit once
> GET_FEATURES reports it as being unsupported on the host.
> 
> This is to avoid a bug where ->handle_kicks() are missed when EVENT_IDX
> is enabled by default in userspace code.
> 
> (mst: Rename to VHOST_SCSI_FEATURES add comment)
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Asias He <asias@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Reviewed-by: Asias He <asias@redhat.com>

> ---
>  drivers/vhost/tcm_vhost.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> index 0524267..4a68c1f 100644
> --- a/drivers/vhost/tcm_vhost.c
> +++ b/drivers/vhost/tcm_vhost.c
> @@ -60,6 +60,15 @@ enum {
>  	VHOST_SCSI_VQ_IO = 2,
>  };
>  
> +/*
> + * VIRTIO_RING_F_EVENT_IDX seems broken. Not sure the bug is in
> + * kernel but disabling it helps.
> + * TODO: debug and remove the workaround.
> + */
> +enum {
> +	VHOST_SCSI_FEATURES = VHOST_FEATURES & (~VIRTIO_RING_F_EVENT_IDX)
> +};
> +
>  #define VHOST_SCSI_MAX_TARGET	256
>  #define VHOST_SCSI_MAX_VQ	128
>  
> @@ -981,7 +990,7 @@ static void vhost_scsi_flush(struct vhost_scsi *vs)
>  
>  static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
>  {
> -	if (features & ~VHOST_FEATURES)
> +	if (features & ~VHOST_SCSI_FEATURES)
>  		return -EOPNOTSUPP;
>  
>  	mutex_lock(&vs->dev.mutex);
> @@ -1027,7 +1036,7 @@ static long vhost_scsi_ioctl(struct file *f, unsigned int ioctl,
>  			return -EFAULT;
>  		return 0;
>  	case VHOST_GET_FEATURES:
> -		features = VHOST_FEATURES;
> +		features = VHOST_SCSI_FEATURES;
>  		if (copy_to_user(featurep, &features, sizeof features))
>  			return -EFAULT;
>  		return 0;
> -- 
> 1.7.2.5
> 

-- 
Asias