From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VDq9X-00044m-D7 for qemu-devel@nongnu.org; Mon, 26 Aug 2013 02:19:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VDq9R-0005Pu-0I for qemu-devel@nongnu.org; Mon, 26 Aug 2013 02:19:51 -0400 Message-ID: <1377497959.3819.36.camel@pasglop> From: Benjamin Herrenschmidt Date: Mon, 26 Aug 2013 16:19:19 +1000 In-Reply-To: <87vc2tysur.fsf@linux.vnet.ibm.com> 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> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: Alexey Kardashevskiy , Paolo Bonzini , qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org On Mon, 2013-08-26 at 10:02 +0530, Nikunj A Dadhania wrote: > > From: Nikunj A Dadhania > > 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 > --- > hw/scsi/spapr_vscsi.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c > index e9090e5..fae3644 100644 > --- a/hw/scsi/spapr_vscsi.c > +++ b/hw/scsi/spapr_vscsi.c > @@ -858,6 +858,40 @@ 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 = 0; The above initialization isn't useful > + int rc = true; > + > + vcap = &req->iu.mad.capabilities; > + len = be16_to_cpu(vcap->common.length); > + if (len > sizeof(&cap)) { ^ Ugh ? Why the & here ? > + fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n"); > + goto error_out; > + } I am not 100% familiar with the protocol, could it be that we should just read sizeof(cap) instead of erroring out or is there no way it can be correct and have a len too long ? > + rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), > + &cap, len); > + if (rc) { > + fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n"); > + } > + > + 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, be64_to_cpu(vcap->buffer), > + &cap, len); > + if (rc) { > + fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n"); > + } > +error_out: > + 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 +912,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));