From patchwork Thu Mar 28 00:21:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 231861 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 380E42C009F for ; Thu, 28 Mar 2013 11:21:53 +1100 (EST) Received: from localhost ([::1]:50620 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL0bH-0000YS-Ad for incoming@patchwork.ozlabs.org; Wed, 27 Mar 2013 20:21:51 -0400 Received: from eggs.gnu.org ([208.118.235.92]:50976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL0b1-0000YA-NE for qemu-devel@nongnu.org; Wed, 27 Mar 2013 20:21:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UL0b0-0001kK-AT for qemu-devel@nongnu.org; Wed, 27 Mar 2013 20:21:35 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:45728 helo=linux-iscsi.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL0b0-0001kE-3z for qemu-devel@nongnu.org; Wed, 27 Mar 2013 20:21:34 -0400 Received: from [192.168.1.68] (75-37-193-228.lightspeed.lsatca.sbcglobal.net [75.37.193.228]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id 8A26F22D9D0; Thu, 28 Mar 2013 00:10:30 +0000 (UTC) From: "Nicholas A. Bellinger" To: "Michael S. Tsirkin" In-Reply-To: <1364426662.17698.41.camel@haakon2.linux-iscsi.org> References: <1364421586-29857-1-git-send-email-nab@linux-iscsi.org> <1364421586-29857-3-git-send-email-nab@linux-iscsi.org> <20130327222856.GC12518@redhat.com> <1364424342.17698.31.camel@haakon2.linux-iscsi.org> <20130327225021.GA12858@redhat.com> <20130327225349.GA12927@redhat.com> <1364426662.17698.41.camel@haakon2.linux-iscsi.org> Date: Wed, 27 Mar 2013 17:21:30 -0700 Message-ID: <1364430090.17698.50.camel@haakon2.linux-iscsi.org> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 67.23.28.174 Cc: kvm-devel , qemu-devel , lf-virt , Anthony Liguori , target-devel , Stefan Hajnoczi , Paolo Bonzini , Asias He Subject: Re: [Qemu-devel] [PATCH 2/3] vhost-scsi: new device supporting the tcm_vhost Linux kernel module X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Wed, 2013-03-27 at 16:24 -0700, Nicholas A. Bellinger wrote: > On Thu, 2013-03-28 at 00:53 +0200, Michael S. Tsirkin wrote: > > On Thu, Mar 28, 2013 at 12:50:21AM +0200, Michael S. Tsirkin wrote: > > > On Wed, Mar 27, 2013 at 03:45:42PM -0700, Nicholas A. Bellinger wrote: > > > > On Thu, 2013-03-28 at 00:28 +0200, Michael S. Tsirkin wrote: > > > > > On Wed, Mar 27, 2013 at 09:59:45PM +0000, Nicholas A. Bellinger wrote: > > > > > > From: Paolo Bonzini > > > > > > > > > > > > The WWPN specified in configfs is passed to "-device vhost-scsi-pci". > > > > > > The tgpt field of the SET_ENDPOINT ioctl is obsolete now, so it is not > > > > > > available from the QEMU command-line. Instead, I hardcode it to zero. > > > > > > > > > > > > Changes in V4: > > > > > > - Set event_idx=off by default (nab, thanks asias) > > > > > > > > > > Why? What's going on here? > > > > > > > > > > > > > Not disabling event_idx by default, or disabling from the command line > > > > ends up resulting in ->handle_kick() not getting called for subsequent > > > > commands.. > > > > > > > > I spent some time trying to track this down recently with no luck, and > > > > AFAICT it's always been required in order for vhost-scsi to function. > > > > > > Hmm this is a bug in kernel then. A better work-around is > > > to disable EVENT_IDX in kernel. Let's do it for 3.9? > > > > But before we do, can you check that SET_FEATURES is > > called with this bit set if you enable event_idx? > > If not that's your bug then ... > > > > Ok, SET_FEATURES is currently not setting any bits at all based upon > vhost_dev->features, so it looks like a vhost-scsi-pci bug.. > > Adding the following patch for a vhost_scsi specific ->get_features() call in order to strip off the unsupported host feature bits. Please review. Also, providing the proper EVENT_IDX bit to SET_FEATURES results in the same lost ->handle_kick() for vhost-scsi-pci devices, so the previous VHOST_TCM_FEATURES patch to strip off EVENT_IDX is still required to work.. Sending this out separately, please review and ACK. Thanks, --nab diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c index 70e42fc..adc929b 100644 --- a/hw/vhost-scsi.c +++ b/hw/vhost-scsi.c @@ -128,6 +128,28 @@ static void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev) vhost_dev_disable_notifiers(&vs->dev, vdev); } +static uint32_t vhost_scsi_get_features(VirtIODevice *vdev, + uint32_t features) +{ + VHostSCSI *vs = (VHostSCSI *)vdev; + + /* Clear features not supported by host kernel. */ + if (!(vs->dev.features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))) { + features &= ~(1 << VIRTIO_F_NOTIFY_ON_EMPTY); + } + if (!(vs->dev.features & (1 << VIRTIO_RING_F_INDIRECT_DESC))) { + features &= ~(1 << VIRTIO_RING_F_INDIRECT_DESC); + } + if (!(vs->dev.features & (1 << VIRTIO_RING_F_EVENT_IDX))) { + features &= ~(1 << VIRTIO_RING_F_EVENT_IDX); + } + if (!(vs->dev.features & (1 << VIRTIO_SCSI_F_HOTPLUG))) { + features &= ~(1 << VIRTIO_SCSI_F_HOTPLUG); + } + + return features; +} + static void vhost_scsi_set_config(VirtIODevice *vdev, const uint8_t *config) { @@ -203,6 +225,7 @@ VirtIODevice *vhost_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) vs = (VHostSCSI *)virtio_scsi_init_common(dev, proxyconf, sizeof(VHostSCSI)); + vs->vs.vdev.get_features = vhost_scsi_get_features; vs->vs.vdev.set_config = vhost_scsi_set_config; vs->vs.vdev.set_status = vhost_scsi_set_status; vs->vs.vdev.guest_notifier_mask = vhost_scsi_guest_notifier_mask; @@ -219,7 +242,7 @@ VirtIODevice *vhost_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) return NULL; } vs->dev.backend_features = 0; - vs->dev.acked_features = 0; + vs->dev.acked_features = vs->dev.features; error_setg(&vs->migration_blocker, "vhost-scsi does not support migration"); diff --git a/hw/vhost-scsi.h b/hw/vhost-scsi.h index b01f012..70f4a58 100644 --- a/hw/vhost-scsi.h +++ b/hw/vhost-scsi.h @@ -50,14 +50,12 @@ enum vhost_scsi_vq_list { #define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, int) #define DEFINE_VHOST_SCSI_PROPERTIES(_state, _features_field, _conf_field) \ - DEFINE_PROP_BIT("indirect_desc", _state, _features_field, VIRTIO_RING_F_INDIRECT_DESC, true), \ - DEFINE_PROP_BIT("event_idx", _state, _features_field, VIRTIO_RING_F_EVENT_IDX, false), \ + DEFINE_VIRTIO_COMMON_FEATURES(_state, _features_field), \ DEFINE_PROP_STRING("vhostfd", _state, _conf_field.vhostfd), \ DEFINE_PROP_STRING("wwpn", _state, _conf_field.wwpn), \ DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \ DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF), \ - DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128), \ - DEFINE_PROP_BIT("hotplug", _state, _features_field, VIRTIO_SCSI_F_HOTPLUG, true) + DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128) VirtIODevice *vhost_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf); void vhost_scsi_exit(VirtIODevice *vdev); diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 9dc7150..c59e9c6 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -595,7 +595,6 @@ VirtIOSCSICommon *virtio_scsi_init_common(DeviceState *dev, VirtIOSCSIConf *prox s->cdb_size = VIRTIO_SCSI_CDB_SIZE; s->vdev.get_config = virtio_scsi_get_config; - s->vdev.get_features = virtio_scsi_get_features; s->ctrl_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE, virtio_scsi_handle_ctrl); @@ -618,6 +617,7 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) s = (VirtIOSCSI *)virtio_scsi_init_common(dev, proxyconf, sizeof(VirtIOSCSI)); + s->vs.vdev.get_features = virtio_scsi_get_features; s->vs.vdev.set_config = virtio_scsi_set_config; s->vs.vdev.reset = virtio_scsi_reset;