From patchwork Thu Jan 31 11:12:29 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 217160 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 C12492C0040 for ; Thu, 31 Jan 2013 22:08:33 +1100 (EST) Received: from localhost ([::1]:35055 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U0s0N-0007mJ-LB for incoming@patchwork.ozlabs.org; Thu, 31 Jan 2013 06:08:31 -0500 Received: from eggs.gnu.org ([208.118.235.92]:41373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U0s0F-0007lj-2o for qemu-devel@nongnu.org; Thu, 31 Jan 2013 06:08:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1U0s0B-0008Ob-6A for qemu-devel@nongnu.org; Thu, 31 Jan 2013 06:08:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:9137) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U0s0A-0008OL-VF for qemu-devel@nongnu.org; Thu, 31 Jan 2013 06:08:19 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r0VB8GNc028279 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 31 Jan 2013 06:08:16 -0500 Received: from redhat.com (vpn1-5-97.ams2.redhat.com [10.36.5.97]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id r0VB8Dkd016979; Thu, 31 Jan 2013 06:08:13 -0500 Date: Thu, 31 Jan 2013 13:12:29 +0200 From: "Michael S. Tsirkin" To: Paolo Bonzini Message-ID: <20130131111228.GB520@redhat.com> References: <1359564086-19705-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1359564086-19705-1-git-send-email-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: asias@redhat.com, qemu-devel@nongnu.org, nab@linux-iscsi.org, stefanha@linux.vnet.ibm.com Subject: Re: [Qemu-devel] [PATCH WIP 0/4] 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, Jan 30, 2013 at 05:41:22PM +0100, Paolo Bonzini wrote: > Ok, so here is my attempt at a vhost-scsi device. I'm creating an > entirely separate device, with the common parts of virtio-scsi and > vhost-scsi (actually little more than the initialization) grouped into > a VirtIOSCSICommon type. The device is used simply like "-device > vhost-scsi-pci,wwpn=WWPN", with all configuration done in configfs > beforehand. > > As expected, using a separate device finds a snag: vhost-scsi is passing > force=false to vhost_dev_init, and the BIOS does not use MSI-X so it > will actually use the non-vhost implementation which is wrong. I fixed > this by passing force=true; I'm not sure what that would break, but I > figured "not much" since the BIOS polls and does not rely on interrupts. > > That makes vhost start, but it still doesn't work for me with a 3.7.2 > kernel on the host. Even Nick's patches hang the guest as soon as vhost > starts, and I get the same behavior with mine. (Of course with my > patches the BIOS hangs and you never reach Linux; but try a BIOS without > virtio-scsi support, and you'll see Linux hanging in the same way). > > Here is my configuration: > > cd /sys/kernel/config/target > mkdir -p core/fileio_0/fileio > echo 'fd_dev_name=/home/pbonzini/test.img,fd_dev_size=5905580032' > core/fileio_0/fileio/control > echo 1 > core/fileio_0/fileio/enable > mkdir -p vhost/naa.600140554cf3a18e/tpgt_0/lun/lun_0 > cd vhost/naa.600140554cf3a18e/tpgt_0 > ln -sf ../../../../../core/fileio_0/fileio/ lun/lun_0/virtual_scsi_port > echo naa.60014053226f0388 > nexus > > Nick's patches are run with "-vhost-scsi id=vs,tpgt=0,wwpn=naa.600140554cf3a18e > -device virtio-scsi-pci,vhost-scsi=vs". Perhaps I'm doing something wrong. > > Another small bug I found is an ordering problem between > VHOST_SET_VRING_KICK and VHOST_SCSI_SET_ENDPOINT. Starting the vq > causes a "vhost_scsi_handle_vq endpoint not set" error in dmesg. > Because of this I added the first two patches, which let me do > VHOST_SCSI_SET_ENDPOINT before VHOST_SET_VRING_KICK but after setting > up the vring. > > Unfortunately, this is not enough to fix the hang. And anyway, it's > probably simpler to avoid the two patches and remove this test from the > tcm_vhost.c vhost_scsi_set/clear_endpoint functions: > > mutex_lock(&vs->dev.mutex); > /* Verify that ring has been setup correctly. */ > for (index = 0; index < vs->dev.nvqs; ++index) { > /* Verify that ring has been setup correctly. */ > if (!vhost_vq_access_ok(&vs->vqs[index])) { > mutex_unlock(&vs->dev.mutex); > return -EFAULT; > } > } > mutex_unlock(&vs->dev.mutex); Well userspace should initialize the kick eventfd to 0, it seems to init it to 1 which is why we get the error. But I think the only issue is pr_err: vhost-net already ignores such a kick with no backend. So let's just remove it, preferably for 3.8. ---> tcm_vhost: fix pr_err on early kick It's OK to get kick before backend is set or after it is cleared, we can just ignore it. Signed-off-by: Michael S. Tsirkin diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c index b20df5c..22321cf 100644 --- a/drivers/vhost/tcm_vhost.c +++ b/drivers/vhost/tcm_vhost.c @@ -575,10 +575,8 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */ tv_tpg = vs->vs_tpg; - if (unlikely(!tv_tpg)) { - pr_err("%s endpoint not set\n", __func__); + if (unlikely(!tv_tpg)) return; - } mutex_lock(&vq->mutex); vhost_disable_notify(&vs->dev, vq);