From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48778) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3giV-0001Bx-CX for qemu-devel@nongnu.org; Mon, 29 Jul 2013 02:14:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V3giP-0005uB-6X for qemu-devel@nongnu.org; Mon, 29 Jul 2013 02:13:59 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:62320) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3giO-0005tk-TO for qemu-devel@nongnu.org; Mon, 29 Jul 2013 02:13:53 -0400 Received: by mail-pa0-f47.google.com with SMTP id kl13so5452414pab.6 for ; Sun, 28 Jul 2013 23:13:52 -0700 (PDT) Message-ID: <51F6081A.70104@ozlabs.ru> Date: Mon, 29 Jul 2013 16:13:46 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1375073369-17546-1-git-send-email-aik@ozlabs.ru> <51F606EA.3090409@redhat.com> In-Reply-To: <51F606EA.3090409@redhat.com> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf On 07/29/2013 04:08 PM, Paolo Bonzini wrote: > Il 29/07/2013 06:49, Alexey Kardashevskiy ha scritto: >> At the moment the guest kernel issues two types of task management >> requests to the hypervisor - task about and lun reset. This adds >> handling for these tasks. As spapr-vscsi starts calling scsi_req_cancel(), >> free_request callback was implemented. >> >> As virtio-vscsi, spapr-vscsi does not handle CLEAR_ACA either as CDB >> control byte does not seem to be used at all so NACA bit is not >> set to the guest so the guest has no good reason to call CLEAR_ACA task. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> Changes: >> 2013/07/29 (v5): >> * sending response on task management was fixed not to use vscsi_send_rsp() >> as it is unable to send response data >> >> 2013/07/26: >> * fixed error handling >> >> 2013/07/23: >> * remove unnecessary free_request callback >> >> 2013/07/22: >> * fixed LUN_RESET (it used to clear requests while it should reset a device) >> * added handling of ABORT_TASK_SET/CLEAR_TASK_SET >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/scsi/spapr_vscsi.c | 117 ++++++++++++++++++++++++++++++++++++++------------ >> hw/scsi/srp.h | 7 +++ >> 2 files changed, 96 insertions(+), 28 deletions(-) >> >> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c >> index 541ffcc..8e49d0d 100644 >> --- a/hw/scsi/spapr_vscsi.c >> +++ b/hw/scsi/spapr_vscsi.c >> @@ -118,6 +118,20 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s) >> return NULL; >> } >> >> +static struct vscsi_req *vscsi_find_req(VSCSIState *s, uint64_t srp_tag) >> +{ >> + vscsi_req *req; >> + int i; >> + >> + for (i = 0; i < VSCSI_REQ_LIMIT; i++) { >> + req = &s->reqs[i]; >> + if (req->iu.srp.cmd.tag == srp_tag) { >> + return req; >> + } >> + } >> + return NULL; >> +} >> + >> static void vscsi_put_req(vscsi_req *req) >> { >> if (req->sreq != NULL) { >> @@ -654,40 +668,87 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req) >> static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req) >> { >> union viosrp_iu *iu = &req->iu; >> - int fn; >> + vscsi_req *tmpreq; >> + int i, lun = 0, resp = SRP_TSK_MGMT_COMPLETE; >> + SCSIDevice *d; >> + uint64_t tag = iu->srp.rsp.tag; >> + uint8_t sol_not = iu->srp.cmd.sol_not; >> >> fprintf(stderr, "vscsi_process_tsk_mgmt %02x\n", >> iu->srp.tsk_mgmt.tsk_mgmt_func); >> >> - switch (iu->srp.tsk_mgmt.tsk_mgmt_func) { >> -#if 0 /* We really don't deal with these for now */ >> - case SRP_TSK_ABORT_TASK: >> - fn = ABORT_TASK; >> - break; >> - case SRP_TSK_ABORT_TASK_SET: >> - fn = ABORT_TASK_SET; >> - break; >> - case SRP_TSK_CLEAR_TASK_SET: >> - fn = CLEAR_TASK_SET; >> - break; >> - case SRP_TSK_LUN_RESET: >> - fn = LOGICAL_UNIT_RESET; >> - break; >> - case SRP_TSK_CLEAR_ACA: >> - fn = CLEAR_ACA; >> - break; >> -#endif >> - default: >> - fn = 0; >> - } >> - if (fn) { >> - /* XXX Send/Handle target task management */ >> - ; >> + d = vscsi_device_find(&s->bus, be64_to_cpu(req->iu.srp.tsk_mgmt.lun), &lun); >> + if (!d) { >> + resp = SRP_TSK_MGMT_FIELDS_INVALID; >> } else { >> - vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0); >> - vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0); >> + switch (iu->srp.tsk_mgmt.tsk_mgmt_func) { >> + case SRP_TSK_ABORT_TASK: >> + if (d->lun != lun) { >> + resp = SRP_TSK_MGMT_FIELDS_INVALID; >> + break; >> + } >> + >> + tmpreq = vscsi_find_req(s, req->iu.srp.tsk_mgmt.task_tag); >> + if (tmpreq && tmpreq->sreq) { >> + assert(tmpreq->sreq->hba_private); >> + scsi_req_cancel(tmpreq->sreq); >> + } >> + break; >> + >> + case SRP_TSK_LUN_RESET: >> + if (d->lun != lun) { >> + resp = SRP_TSK_MGMT_FIELDS_INVALID; >> + break; >> + } >> + >> + qdev_reset_all(&d->qdev); >> + break; >> + >> + case SRP_TSK_ABORT_TASK_SET: >> + case SRP_TSK_CLEAR_TASK_SET: >> + if (d->lun != lun) { >> + resp = SRP_TSK_MGMT_FIELDS_INVALID; >> + break; >> + } >> + >> + for (i = 0; i < VSCSI_REQ_LIMIT; i++) { >> + tmpreq = &s->reqs[i]; >> + if (tmpreq->iu.srp.cmd.lun != req->iu.srp.tsk_mgmt.lun) { >> + continue; >> + } >> + if (!tmpreq->active || !tmpreq->sreq) { >> + continue; >> + } >> + assert(tmpreq->sreq->hba_private); >> + scsi_req_cancel(tmpreq->sreq); >> + } >> + break; >> + >> + case SRP_TSK_CLEAR_ACA: >> + resp = SRP_TSK_MGMT_NOT_SUPPORTED; >> + break; >> + >> + default: >> + resp = SRP_TSK_MGMT_FIELDS_INVALID; >> + break; >> + } >> } >> - return !fn; >> + >> + /* Compose the response here as */ >> + memset(iu, 0, sizeof(struct srp_rsp) + 4); >> + iu->srp.rsp.opcode = SRP_RSP; >> + iu->srp.rsp.req_lim_delta = cpu_to_be32(1); >> + iu->srp.rsp.tag = tag; >> + iu->srp.rsp.flags |= SRP_RSP_FLAG_RSPVALID; >> + iu->srp.rsp.resp_data_len = cpu_to_be32(4); >> + /* Copy UCSOLNT bit as SRP_RSP_FLAG_RSPVALID is set */ >> + iu->srp.rsp.sol_not = (sol_not & 0x04) >> 1; > > This is ">> 2" and also needs to look at SCSOLNT with the same rules as > normal responses, but I can fix this when applying. Ouch, overlooked :( Thanks! -- Alexey