From patchwork Tue Aug 27 05:43:09 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nikunj A Dadhania X-Patchwork-Id: 270020 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 7B2FC2C00A2 for ; Tue, 27 Aug 2013 15:43:59 +1000 (EST) Received: from localhost ([::1]:54230 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEC4L-0004Nm-1a for incoming@patchwork.ozlabs.org; Tue, 27 Aug 2013 01:43:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42105) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEC3z-0004NX-Ne for qemu-devel@nongnu.org; Tue, 27 Aug 2013 01:43:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEC3r-0000t5-3o for qemu-devel@nongnu.org; Tue, 27 Aug 2013 01:43:35 -0400 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:34535) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEC3q-0000sb-Gi for qemu-devel@nongnu.org; Tue, 27 Aug 2013 01:43:27 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 27 Aug 2013 15:40:00 +1000 Received: from d23dlp03.au.ibm.com (202.81.31.214) by e23smtp08.au.ibm.com (202.81.31.205) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 27 Aug 2013 15:39:58 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id E04D63578052; Tue, 27 Aug 2013 15:43:13 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7R5R80I8782294; Tue, 27 Aug 2013 15:27:09 +1000 Received: from d23av01.au.ibm.com (localhost [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7R5hDZ0006528; Tue, 27 Aug 2013 15:43:13 +1000 Received: from abhimanyu.in.ibm.com.vnet.linux.ibm.com (abhimanyu.in.ibm.com [9.124.35.152] (may be forged)) by d23av01.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id r7R5hAef006422 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Tue, 27 Aug 2013 15:43:11 +1000 From: Nikunj A Dadhania To: Alexander Graf In-Reply-To: <421F6761-C72A-4BFD-9164-BFFB86E852A0@suse.de> References: <1377249737-12570-1-git-send-email-aik@ozlabs.ru> <24C2B209-2082-4AF8-A8FB-1FF8A8B7751B@suse.de> <1377468637.3819.27.camel@pasglop> <87vc2tysur.fsf@linux.vnet.ibm.com> <874naczpk5.fsf@linux.vnet.ibm.com> <011735F8-0ED7-40CF-A23A-CE34C21F1D3C@suse.de> <87wqn8y8ra.fsf@linux.vnet.ibm.com> <421F6761-C72A-4BFD-9164-BFFB86E852A0@suse.de> User-Agent: Notmuch/0.14+104~g0a21fb9 (http://notmuchmail.org) Emacs/24.3.50.1 (x86_64-unknown-linux-gnu) Date: Tue, 27 Aug 2013 11:13:09 +0530 Message-ID: <87y57nadua.fsf@abhimanyu.in.ibm.com> MIME-Version: 1.0 X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13082705-5140-0000-0000-000003BB5093 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 202.81.31.141 Cc: Alexey Kardashevskiy , Paolo Bonzini , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities 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 Alexander Graf writes: > On 26.08.2013, at 13:46, Nikunj A Dadhania wrote: > >> Alexander Graf writes: >> >>> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote: >>> >>>> This implements capabilities exchange between host and client. >>>> As at the moment no capability is supported, put zero flags everywhere >>>> and return. >>>> >>>> Signed-off-by: Nikunj A Dadhania >>>> --- >>>> + rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len); >>>> + if (rc) { >>>> + fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n"); >>> >>> At this point cap contains random host data, no? >> >> Yes, and we zero it out in this case. > > Then please make this obvious to the reader. Either memset(0) it or do > cap = { };. But do something that doesn't make me look up the header > file to check whether you really did catch all the fields there are in > the struct. Here is the patch incorporating the comments, moreover goto is not there anymore, we are being more accomodative and copying only the size(cap) structure even if guest is requesting for more. From: Nikunj A Dadhania This implements capabilities exchange between vscsi host and client. As at the moment no capability is supported, put zero flags everywhere and return. Signed-off-by: Nikunj A Dadhania --- hw/scsi/spapr_vscsi.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index e9090e5..030b775 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -858,6 +858,57 @@ static int vscsi_send_adapter_info(VSCSIState *s, vscsi_req *req) return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT); } +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req) +{ + struct viosrp_capabilities *vcap; + struct capabilities cap = { }; + uint16_t len, req_len; + uint64_t buffer; + int rc; + + vcap = &req->iu.mad.capabilities; + req_len = len = be16_to_cpu(vcap->common.length); + buffer = be64_to_cpu(vcap->buffer); + if (len > sizeof(cap)) { + fprintf(stderr, "vscsi_send_capabilities: capabilities size mismatch !\n"); + + /* + * Just read and populate the structure that is known. + * Zero rest of the structure. + */ + len = sizeof(cap); + } + rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len); + if (rc) { + fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n"); + } + + /* + * Current implementation does not suppport any migration or + * reservation capabilities. Construct the response telling the + * guest not to use them. + */ + cap.flags = 0; + cap.migration.ecl = 0; + cap.reserve.type = 0; + cap.migration.common.server_support = 0; + cap.reserve.common.server_support = 0; + + rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len); + if (rc) { + fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n"); + } + if (req_len > len) { + /* + * Being paranoid and lets not worry about the error code + * here. Actual write of the cap is done above. + */ + spapr_vio_dma_set(&s->vdev, (buffer + len), 0, (req_len - len)); + } + vcap->common.status = rc ? cpu_to_be32(1) : 0; + return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT); +} + static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) { union mad_iu *mad = &req->iu.mad; @@ -878,6 +929,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) mad->host_config.common.status = cpu_to_be16(1); vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT); break; + case VIOSRP_CAPABILITIES_TYPE: + vscsi_send_capabilities(s, req); + break; default: fprintf(stderr, "VSCSI: Unknown MAD type %02x\n", be32_to_cpu(mad->empty_iu.common.type));