| Message ID | 1374830452-5365-1-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
Il 26/07/2013 11:20, Alexey Kardashevskiy ha scritto: > - return !fn; > + > + vscsi_send_rsp(s, req, resp, 0, 0); We're not there yet, but getting closer... These response codes are _not_ SCSI status codes that go in the SRP_RSP's status field. They go (in the error case) in the "response data" field, so you have to set RSPVALID and set the first four bytes of the data[] array in the format that I mentioned before. The status field must be zero, because the spec says: "Response data shall not be provided in any SRP_RSP response that returns a non-zero status code in the STATUS field". Paolo > + return 1; > } > > static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req) > diff --git a/hw/scsi/srp.h b/hw/scsi/srp.h > index 5e0cad5..d27f31d 100644 > --- a/hw/scsi/srp.h > +++ b/hw/scsi/srp.h > @@ -90,6 +90,13 @@ enum { > SRP_REV16A_IB_IO_CLASS = 0x0100 > }; > > +enum { > + SRP_TSK_MGMT_COMPLETE = 0x00, > + SRP_TSK_MGMT_FIELDS_INVALID = 0x02, > + SRP_TSK_MGMT_NOT_SUPPORTED = 0x04, > + SRP_TSK_MGMT_FAILED = 0x05 > +}; > + > struct srp_direct_buf { > uint64_t va; > uint32_t key; >
On 07/26/2013 07:34 PM, Paolo Bonzini wrote: > Il 26/07/2013 11:20, Alexey Kardashevskiy ha scritto: >> - return !fn; >> + >> + vscsi_send_rsp(s, req, resp, 0, 0); > > We're not there yet, but getting closer... Oh. Right. Sorry and thanks (ohci + ehci + vscsi at the same time and my mind melted :) ) Is this better? - vscsi_send_rsp(s, req, resp, 0, 0); + 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); + iu->srp.rsp.sol_not = (sol_not & 0x02) >> 1; + iu->srp.rsp.status = GOOD; + iu->srp.rsp.data[3] = resp; + + vscsi_send_iu(s, req, sizeof(iu->srp.rsp) + 4, VIOSRP_SRP_FORMAT); And it seems I found a bug in vscsi_send_rsp() (sol_not is always zero), heh. > These response codes are _not_ SCSI status codes that go in the > SRP_RSP's status field. They go (in the error case) in the "response > data" field, so you have to set RSPVALID and set the first four bytes of > the data[] array in the format that I mentioned before. > > The status field must be zero, because the spec says: "Response data > shall not be provided in any SRP_RSP response that returns a non-zero > status code in the STATUS field". > > Paolo > >> + return 1; >> } >> >> static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req) >> diff --git a/hw/scsi/srp.h b/hw/scsi/srp.h >> index 5e0cad5..d27f31d 100644 >> --- a/hw/scsi/srp.h >> +++ b/hw/scsi/srp.h >> @@ -90,6 +90,13 @@ enum { >> SRP_REV16A_IB_IO_CLASS = 0x0100 >> }; >> >> +enum { >> + SRP_TSK_MGMT_COMPLETE = 0x00, >> + SRP_TSK_MGMT_FIELDS_INVALID = 0x02, >> + SRP_TSK_MGMT_NOT_SUPPORTED = 0x04, >> + SRP_TSK_MGMT_FAILED = 0x05 >> +}; >> + >> struct srp_direct_buf { >> uint64_t va; >> uint32_t key; >> >
Il 26/07/2013 12:25, Alexey Kardashevskiy ha scritto: > On 07/26/2013 07:34 PM, Paolo Bonzini wrote: >> Il 26/07/2013 11:20, Alexey Kardashevskiy ha scritto: >>> - return !fn; >>> + >>> + vscsi_send_rsp(s, req, resp, 0, 0); >> >> We're not there yet, but getting closer... > > Oh. Right. Sorry and thanks (ohci + ehci + vscsi at the same time and my > mind melted :) ) > > Is this better? > > - vscsi_send_rsp(s, req, resp, 0, 0); > + 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); > + iu->srp.rsp.sol_not = (sol_not & 0x02) >> 1; > + iu->srp.rsp.status = GOOD; > + iu->srp.rsp.data[3] = resp; > + > + vscsi_send_iu(s, req, sizeof(iu->srp.rsp) + 4, VIOSRP_SRP_FORMAT); > > And it seems I found a bug in vscsi_send_rsp() (sol_not is always zero), heh. I cannot see the definition of sol_not, here is what the spec says though: The solicited notification (SOLNT) bit indicates whether the SRP initiator port specified normal or solicited message reception notification for this response. If the STATUS field is non-zero or if the RSP_CODE field is present and non-zero, then the SOLNT bit shall contain the value that was specified in the UCSOLNT bit of the corresponding SRP_CMD or SRP_TSK_MGMT request; otherwise, the SOLNT bit shall contain the value that was specified in the SCSOLNT bit of the corresponding SRP_CMD or SRP_TSK_MGMT request. So it must be bit 1 (SCSOLNT) or bit 2 (UCSOLNT) depending on whether resp is zero or non-zero (respectively). Paolo > > >> These response codes are _not_ SCSI status codes that go in the >> SRP_RSP's status field. They go (in the error case) in the "response >> data" field, so you have to set RSPVALID and set the first four bytes of >> the data[] array in the format that I mentioned before. >> >> The status field must be zero, because the spec says: "Response data >> shall not be provided in any SRP_RSP response that returns a non-zero >> status code in the STATUS field". >> >> Paolo >> >>> + return 1; >>> } >>> >>> static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req) >>> diff --git a/hw/scsi/srp.h b/hw/scsi/srp.h >>> index 5e0cad5..d27f31d 100644 >>> --- a/hw/scsi/srp.h >>> +++ b/hw/scsi/srp.h >>> @@ -90,6 +90,13 @@ enum { >>> SRP_REV16A_IB_IO_CLASS = 0x0100 >>> }; >>> >>> +enum { >>> + SRP_TSK_MGMT_COMPLETE = 0x00, >>> + SRP_TSK_MGMT_FIELDS_INVALID = 0x02, >>> + SRP_TSK_MGMT_NOT_SUPPORTED = 0x04, >>> + SRP_TSK_MGMT_FAILED = 0x05 >>> +}; >>> + >>> struct srp_direct_buf { >>> uint64_t va; >>> uint32_t key; >>> >> > >
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index 46f4455..2c09a88 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -117,6 +117,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) { @@ -753,40 +767,73 @@ 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; 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; + + vscsi_send_rsp(s, req, resp, 0, 0); + + return 1; } static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req) diff --git a/hw/scsi/srp.h b/hw/scsi/srp.h index 5e0cad5..d27f31d 100644 --- a/hw/scsi/srp.h +++ b/hw/scsi/srp.h @@ -90,6 +90,13 @@ enum { SRP_REV16A_IB_IO_CLASS = 0x0100 }; +enum { + SRP_TSK_MGMT_COMPLETE = 0x00, + SRP_TSK_MGMT_FIELDS_INVALID = 0x02, + SRP_TSK_MGMT_NOT_SUPPORTED = 0x04, + SRP_TSK_MGMT_FAILED = 0x05 +}; + struct srp_direct_buf { uint64_t va; uint32_t key;
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 <aik@ozlabs.ru> --- Changes: 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 --- hw/scsi/spapr_vscsi.c | 103 ++++++++++++++++++++++++++++++++++++-------------- hw/scsi/srp.h | 7 ++++ 2 files changed, 82 insertions(+), 28 deletions(-)