qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
@ 2013-07-29  4:49 Alexey Kardashevskiy
  2013-07-29  6:08 ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2013-07-29  4:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf, Paolo Bonzini

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/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 <aik@ozlabs.ru>
---
 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;
+    iu->srp.rsp.status = GOOD;
+    iu->srp.rsp.data[3] = resp;
+
+    vscsi_send_iu(s, req, sizeof(iu->srp.rsp) + 4, VIOSRP_SRP_FORMAT);
+
+    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;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
  2013-07-29  4:49 [Qemu-devel] [PATCH v5] spapr-vscsi: add task management Alexey Kardashevskiy
@ 2013-07-29  6:08 ` Paolo Bonzini
  2013-07-29  6:13   ` Alexey Kardashevskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2013-07-29  6:08 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, Alexander Graf

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 <aik@ozlabs.ru>
> ---
> 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 <aik@ozlabs.ru>
> ---
>  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.

Thanks for both patches.

Paolo

> +    iu->srp.rsp.status = GOOD;
> +    iu->srp.rsp.data[3] = resp;
> +
> +    vscsi_send_iu(s, req, sizeof(iu->srp.rsp) + 4, VIOSRP_SRP_FORMAT);
> +
> +    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;
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
  2013-07-29  6:08 ` Paolo Bonzini
@ 2013-07-29  6:13   ` Alexey Kardashevskiy
  2013-08-16  9:45     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2013-07-29  6:13 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-ppc, qemu-devel, 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 <aik@ozlabs.ru>
>> ---
>> 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 <aik@ozlabs.ru>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
  2013-07-29  6:13   ` Alexey Kardashevskiy
@ 2013-08-16  9:45     ` Alexey Kardashevskiy
  2013-08-17  6:21       ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2013-08-16  9:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-ppc, qemu-devel, Alexander Graf

On 07/29/2013 04:13 PM, Alexey Kardashevskiy wrote:
> 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 <aik@ozlabs.ru>
>>> ---
>>> 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 <aik@ozlabs.ru>
>>> ---
>>>  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!


Did this patch go anywhere? I could not find it in
git://github.com/bonzini/qemu.git scsi-next or anywhere else.

Should I repost it as qemu 1.6 was released and we started new cycle?

I am asking because I have 2 more patches about vscsi which depend on this one.

Thank you.


-- 
Alexey

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
  2013-08-16  9:45     ` Alexey Kardashevskiy
@ 2013-08-17  6:21       ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2013-08-17  6:21 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, Alexander Graf

Il 16/08/2013 11:45, Alexey Kardashevskiy ha scritto:
> 
> Did this patch go anywhere? I could not find it in
> git://github.com/bonzini/qemu.git scsi-next or anywhere else.
> 
> Should I repost it as qemu 1.6 was released and we started new cycle?
> 
> I am asking because I have 2 more patches about vscsi which depend on this one.

I will take care of it next week.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-08-17  6:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-29  4:49 [Qemu-devel] [PATCH v5] spapr-vscsi: add task management Alexey Kardashevskiy
2013-07-29  6:08 ` Paolo Bonzini
2013-07-29  6:13   ` Alexey Kardashevskiy
2013-08-16  9:45     ` Alexey Kardashevskiy
2013-08-17  6:21       ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).