From mboxrd@z Thu Jan 1 00:00:00 1970 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 From: Nikunj A Dadhania 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> Date: Tue, 27 Aug 2013 11:13:09 +0530 Message-ID: <87y57nadua.fsf@abhimanyu.in.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Alexey Kardashevskiy , Paolo Bonzini , "qemu-ppc@nongnu.org" , "qemu-devel@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)); -- 1.8.3.1