On Thu, 2013-03-28 at 08:04 +0200, Michael S. Tsirkin wrote:
> On Thu, Mar 28, 2013 at 12:27:10AM +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <[email protected]>
> > 
> > This patch adds a VHOST_TCM_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.
> > 
> > Cc: Michael S. Tsirkin <[email protected]>
> > Cc: Asias He <[email protected]>
> > Cc: Paolo Bonzini <[email protected]>
> > Signed-off-by: Nicholas Bellinger <[email protected]>
> 
> No chance we can debug this properly?
> 

Well, I assumed this was a low priority item ahead of tracking down the
other blocking seabios related issue.

I'm fine with dropping this patch and requiring the event_idx=off option
for now for an initial QEMU merge, but thought this made things a little
easier for CLI users to avoid confusion.

> > ---
> >  drivers/vhost/tcm_vhost.c |   10 ++++++++--
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> > index 0524267..b127edc 100644
> > --- a/drivers/vhost/tcm_vhost.c
> > +++ b/drivers/vhost/tcm_vhost.c
> > @@ -60,6 +60,12 @@ enum {
> >     VHOST_SCSI_VQ_IO = 2,
> >  };
> >  
> > +enum {
> > +   VHOST_TCM_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
> > +                        (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> > +                        (1ULL << VHOST_F_LOG_ALL)
> 
> All the rest of the code uses VHOST_SCSI so rename this too?
> 
> > +};
> > +
> 
> Please do something like
> 
> +/*
> +* 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)
> +}
> 

Patch updated.

Thanks,

--nab

> >  #define VHOST_SCSI_MAX_TARGET      256
> >  #define VHOST_SCSI_MAX_VQ  128
> >  
> > @@ -981,7 +987,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_TCM_FEATURES)
> >             return -EOPNOTSUPP;
> >  
> >     mutex_lock(&vs->dev.mutex);
> > @@ -1027,7 +1033,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_TCM_FEATURES;
> >             if (copy_to_user(featurep, &features, sizeof features))
> >                     return -EFAULT;
> >             return 0;
> > -- 
> > 1.7.2.5


_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to