Patchwork [05/21] scsi: Add 'hba_private' to SCSIRequest

login
register
mail settings
Submitter Kevin Wolf
Date July 19, 2011, 10:15 a.m.
Message ID <1311070524-13533-6-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/105440/
State New
Headers show

Comments

Kevin Wolf - July 19, 2011, 10:15 a.m.
From: Hannes Reinecke <hare@suse.de>

'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
'tag' is still being kept for tracing purposes.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/esp.c          |    2 +-
 hw/lsi53c895a.c   |   22 ++++++++--------------
 hw/scsi-bus.c     |    9 ++++++---
 hw/scsi-disk.c    |    4 ++--
 hw/scsi-generic.c |    5 +++--
 hw/scsi.h         |   10 +++++++---
 hw/spapr_vscsi.c  |   29 +++++++++--------------------
 hw/usb-msd.c      |    9 +--------
 8 files changed, 37 insertions(+), 53 deletions(-)
Anthony Liguori - July 19, 2011, 12:43 p.m.
On 07/19/2011 05:15 AM, Kevin Wolf wrote:
> From: Hannes Reinecke<hare@suse.de>
>
> 'tag' is just an abstraction to identify the command
> from the driver. So we should make that explicit by
> replacing 'tag' with a driver-defined pointer 'hba_private'.
> This saves the lookup for driver handling several commands
> in parallel.
> 'tag' is still being kept for tracing purposes.
>
> Signed-off-by: Hannes Reinecke<hare@suse.de>
> Acked-by: Paolo Bonzini<pbonzini@redhat.com>
> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
> ---
>   hw/esp.c          |    2 +-
>   hw/lsi53c895a.c   |   22 ++++++++--------------
>   hw/scsi-bus.c     |    9 ++++++---
>   hw/scsi-disk.c    |    4 ++--
>   hw/scsi-generic.c |    5 +++--
>   hw/scsi.h         |   10 +++++++---
>   hw/spapr_vscsi.c  |   29 +++++++++--------------------
>   hw/usb-msd.c      |    9 +--------
>   8 files changed, 37 insertions(+), 53 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index aa50800..9ddd637 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
>
>       DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
>       lun = busid&  7;
> -    s->current_req = scsi_req_new(s->current_dev, 0, lun);
> +    s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
>       datalen = scsi_req_enqueue(s->current_req, buf);
>       s->ti_size = datalen;
>       if (datalen != 0) {
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index 940b43a..69eec1d 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
>   static void lsi_request_cancelled(SCSIRequest *req)
>   {
>       LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> -    lsi_request *p;
> +    lsi_request *p = req->hba_private;
>
>       if (s->current&&  req == s->current->req) {
>           scsi_req_unref(req);
> @@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
>           return;
>       }
>
> -    p = lsi_find_by_tag(s, req->tag);
>       if (p) {
>           QTAILQ_REMOVE(&s->queue, p, next);
>           scsi_req_unref(req);
> @@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
>
>   /* Record that data is available for a queued command.  Returns zero if
>      the device was reselected, nonzero if the IO is deferred.  */
> -static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
> +static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
>   {
> -    lsi_request *p;
> -
> -    p = lsi_find_by_tag(s, tag);
> -    if (!p) {
> -        BADF("IO with unknown tag %d\n", tag);
> -        return 1;
> -    }
> +    lsi_request *p = req->hba_private;
>
>       if (p->pending) {
> -        BADF("Multiple IO pending for tag %d\n", tag);
> +        BADF("Multiple IO pending for request %p\n", p);
>       }
>       p->pending = len;
>       /* Reselect if waiting for it, or if reselection triggers an IRQ
> @@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
>       LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
>       int out;
>
> -    if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
> +    if (s->waiting == 1 || !s->current || req->hba_private != s->current ||
>           (lsi_irq_on_rsl(s)&&  !(s->scntl1&  LSI_SCNTL1_CON))) {
> -        if (lsi_queue_tag(s, req->tag, len)) {
> +        if (lsi_queue_req(s, req, len)) {
>               return;
>           }
>       }
> @@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
>       assert(s->current == NULL);
>       s->current = qemu_mallocz(sizeof(lsi_request));
>       s->current->tag = s->select_tag;
> -    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
> +    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
> +                                   s->current);
>
>       n = scsi_req_enqueue(s->current->req, buf);
>       if (n) {
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index ad6a730..8b1a412 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
>       return res;
>   }
>
> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun)
> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> +                            uint32_t lun, void *hba_private)
>   {
>       SCSIRequest *req;
>
> @@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
>       req->dev = d;
>       req->tag = tag;
>       req->lun = lun;
> +    req->hba_private = hba_private;
>       req->status = -1;
>       trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
>       return req;
>   }
>
> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> +                          void *hba_private)
>   {
> -    return d->info->alloc_req(d, tag, lun);
> +    return d->info->alloc_req(d, tag, lun, hba_private);
>   }
>
>   uint8_t *scsi_req_get_buf(SCSIRequest *req)
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index a8c7372..c2a99fe 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
>   static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
>
>   static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
> -        uint32_t lun)
> +                                     uint32_t lun, void *hba_private)
>   {
>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
>       SCSIRequest *req;
>       SCSIDiskReq *r;
>
> -    req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun);
> +    req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun, hba_private);
>       r = DO_UPCAST(SCSIDiskReq, req, req);
>       r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
>       return req;
> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> index 8e59c7e..90345a7 100644
> --- a/hw/scsi-generic.c
> +++ b/hw/scsi-generic.c
> @@ -96,11 +96,12 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
>       return size;
>   }
>
> -static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
> +static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> +                                     void *hba_private)
>   {
>       SCSIRequest *req;
>
> -    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
> +    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun, hba_private);
>       return req;
>   }
>
> diff --git a/hw/scsi.h b/hw/scsi.h
> index c1dca35..6b15bbc 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -43,6 +43,7 @@ struct SCSIRequest {
>       } cmd;
>       BlockDriverAIOCB  *aiocb;
>       bool enqueued;
> +    void *hba_private;
>       QTAILQ_ENTRY(SCSIRequest) next;
>   };
>
> @@ -67,7 +68,8 @@ struct SCSIDeviceInfo {
>       DeviceInfo qdev;
>       scsi_qdev_initfn init;
>       void (*destroy)(SCSIDevice *s);
> -    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
> +    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
> +                              void *hba_private);
>       void (*free_req)(SCSIRequest *req);
>       int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
>       void (*read_data)(SCSIRequest *req);
> @@ -138,8 +140,10 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
>   int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
>   int scsi_sense_valid(SCSISense sense);
>
> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> +                            uint32_t lun, void *hba_private);
> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> +                          void *hba_private);
>   int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
>   void scsi_req_free(SCSIRequest *req);
>   SCSIRequest *scsi_req_ref(SCSIRequest *req);
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index 1c901ef..9e1cb2e 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -121,7 +121,7 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
>       return NULL;
>   }
>
> -static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
> +static void vscsi_put_req(vscsi_req *req)
>   {
>       if (req->sreq != NULL) {
>           scsi_req_unref(req->sreq);

This breaks the build:

make[1]: Nothing to be done for `all'.
   CC    ppc64-softmmu/spapr_vscsi.o
/home/anthony/git/qemu/hw/spapr_vscsi.c: In function 
‘vscsi_command_complete’:
/home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: ‘s’ undeclared 
(first use in this function)
/home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: note: each undeclared 
identifier is reported only once for each function it appears in

This file is only built when libfdt is installed which is probably why 
you didn't catch it.

Ben/David, is there a way we can still build most of this stuff without 
libfdt?  libfdt is still not commonly packaged by some distros.

Regards,

Anthony Liguori
Kevin Wolf - July 19, 2011, 1:06 p.m.
Am 19.07.2011 14:43, schrieb Anthony Liguori:
> On 07/19/2011 05:15 AM, Kevin Wolf wrote:
>> From: Hannes Reinecke<hare@suse.de>
>>
>> 'tag' is just an abstraction to identify the command
>> from the driver. So we should make that explicit by
>> replacing 'tag' with a driver-defined pointer 'hba_private'.
>> This saves the lookup for driver handling several commands
>> in parallel.
>> 'tag' is still being kept for tracing purposes.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> Acked-by: Paolo Bonzini<pbonzini@redhat.com>
>> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
>> ---
>>   hw/esp.c          |    2 +-
>>   hw/lsi53c895a.c   |   22 ++++++++--------------
>>   hw/scsi-bus.c     |    9 ++++++---
>>   hw/scsi-disk.c    |    4 ++--
>>   hw/scsi-generic.c |    5 +++--
>>   hw/scsi.h         |   10 +++++++---
>>   hw/spapr_vscsi.c  |   29 +++++++++--------------------
>>   hw/usb-msd.c      |    9 +--------
>>   8 files changed, 37 insertions(+), 53 deletions(-)
>>
>> diff --git a/hw/esp.c b/hw/esp.c
>> index aa50800..9ddd637 100644
>> --- a/hw/esp.c
>> +++ b/hw/esp.c
>> @@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
>>
>>       DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
>>       lun = busid&  7;
>> -    s->current_req = scsi_req_new(s->current_dev, 0, lun);
>> +    s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
>>       datalen = scsi_req_enqueue(s->current_req, buf);
>>       s->ti_size = datalen;
>>       if (datalen != 0) {
>> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
>> index 940b43a..69eec1d 100644
>> --- a/hw/lsi53c895a.c
>> +++ b/hw/lsi53c895a.c
>> @@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
>>   static void lsi_request_cancelled(SCSIRequest *req)
>>   {
>>       LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
>> -    lsi_request *p;
>> +    lsi_request *p = req->hba_private;
>>
>>       if (s->current&&  req == s->current->req) {
>>           scsi_req_unref(req);
>> @@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
>>           return;
>>       }
>>
>> -    p = lsi_find_by_tag(s, req->tag);
>>       if (p) {
>>           QTAILQ_REMOVE(&s->queue, p, next);
>>           scsi_req_unref(req);
>> @@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
>>
>>   /* Record that data is available for a queued command.  Returns zero if
>>      the device was reselected, nonzero if the IO is deferred.  */
>> -static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
>> +static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
>>   {
>> -    lsi_request *p;
>> -
>> -    p = lsi_find_by_tag(s, tag);
>> -    if (!p) {
>> -        BADF("IO with unknown tag %d\n", tag);
>> -        return 1;
>> -    }
>> +    lsi_request *p = req->hba_private;
>>
>>       if (p->pending) {
>> -        BADF("Multiple IO pending for tag %d\n", tag);
>> +        BADF("Multiple IO pending for request %p\n", p);
>>       }
>>       p->pending = len;
>>       /* Reselect if waiting for it, or if reselection triggers an IRQ
>> @@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
>>       LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
>>       int out;
>>
>> -    if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
>> +    if (s->waiting == 1 || !s->current || req->hba_private != s->current ||
>>           (lsi_irq_on_rsl(s)&&  !(s->scntl1&  LSI_SCNTL1_CON))) {
>> -        if (lsi_queue_tag(s, req->tag, len)) {
>> +        if (lsi_queue_req(s, req, len)) {
>>               return;
>>           }
>>       }
>> @@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
>>       assert(s->current == NULL);
>>       s->current = qemu_mallocz(sizeof(lsi_request));
>>       s->current->tag = s->select_tag;
>> -    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
>> +    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
>> +                                   s->current);
>>
>>       n = scsi_req_enqueue(s->current->req, buf);
>>       if (n) {
>> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
>> index ad6a730..8b1a412 100644
>> --- a/hw/scsi-bus.c
>> +++ b/hw/scsi-bus.c
>> @@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
>>       return res;
>>   }
>>
>> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun)
>> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
>> +                            uint32_t lun, void *hba_private)
>>   {
>>       SCSIRequest *req;
>>
>> @@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
>>       req->dev = d;
>>       req->tag = tag;
>>       req->lun = lun;
>> +    req->hba_private = hba_private;
>>       req->status = -1;
>>       trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
>>       return req;
>>   }
>>
>> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
>> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
>> +                          void *hba_private)
>>   {
>> -    return d->info->alloc_req(d, tag, lun);
>> +    return d->info->alloc_req(d, tag, lun, hba_private);
>>   }
>>
>>   uint8_t *scsi_req_get_buf(SCSIRequest *req)
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index a8c7372..c2a99fe 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
>>   static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
>>
>>   static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
>> -        uint32_t lun)
>> +                                     uint32_t lun, void *hba_private)
>>   {
>>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
>>       SCSIRequest *req;
>>       SCSIDiskReq *r;
>>
>> -    req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun);
>> +    req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun, hba_private);
>>       r = DO_UPCAST(SCSIDiskReq, req, req);
>>       r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
>>       return req;
>> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
>> index 8e59c7e..90345a7 100644
>> --- a/hw/scsi-generic.c
>> +++ b/hw/scsi-generic.c
>> @@ -96,11 +96,12 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
>>       return size;
>>   }
>>
>> -static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
>> +static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
>> +                                     void *hba_private)
>>   {
>>       SCSIRequest *req;
>>
>> -    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
>> +    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun, hba_private);
>>       return req;
>>   }
>>
>> diff --git a/hw/scsi.h b/hw/scsi.h
>> index c1dca35..6b15bbc 100644
>> --- a/hw/scsi.h
>> +++ b/hw/scsi.h
>> @@ -43,6 +43,7 @@ struct SCSIRequest {
>>       } cmd;
>>       BlockDriverAIOCB  *aiocb;
>>       bool enqueued;
>> +    void *hba_private;
>>       QTAILQ_ENTRY(SCSIRequest) next;
>>   };
>>
>> @@ -67,7 +68,8 @@ struct SCSIDeviceInfo {
>>       DeviceInfo qdev;
>>       scsi_qdev_initfn init;
>>       void (*destroy)(SCSIDevice *s);
>> -    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
>> +    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
>> +                              void *hba_private);
>>       void (*free_req)(SCSIRequest *req);
>>       int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
>>       void (*read_data)(SCSIRequest *req);
>> @@ -138,8 +140,10 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
>>   int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
>>   int scsi_sense_valid(SCSISense sense);
>>
>> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
>> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
>> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
>> +                            uint32_t lun, void *hba_private);
>> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
>> +                          void *hba_private);
>>   int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
>>   void scsi_req_free(SCSIRequest *req);
>>   SCSIRequest *scsi_req_ref(SCSIRequest *req);
>> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
>> index 1c901ef..9e1cb2e 100644
>> --- a/hw/spapr_vscsi.c
>> +++ b/hw/spapr_vscsi.c
>> @@ -121,7 +121,7 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
>>       return NULL;
>>   }
>>
>> -static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
>> +static void vscsi_put_req(vscsi_req *req)
>>   {
>>       if (req->sreq != NULL) {
>>           scsi_req_unref(req->sreq);
> 
> This breaks the build:
> 
> make[1]: Nothing to be done for `all'.
>    CC    ppc64-softmmu/spapr_vscsi.o
> /home/anthony/git/qemu/hw/spapr_vscsi.c: In function 
> ‘vscsi_command_complete’:
> /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: ‘s’ undeclared 
> (first use in this function)
> /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: note: each undeclared 
> identifier is reported only once for each function it appears in

Adding Hannes to CC.

> This file is only built when libfdt is installed which is probably why 
> you didn't catch it.

Yes, I did build all targets, but probably don't have most optional
dependencies installed. I guess I should install some more libraries on
that machine.

But as you said, there is no libfdt package in RHEL, so it will not be
among them, and I'd like to avoid installing things from source.

Kevin
Benjamin Herrenschmidt - July 19, 2011, 1:20 p.m.
On Tue, 2011-07-19 at 07:43 -0500, Anthony Liguori wrote:
> 
> This breaks the build:
> 
> make[1]: Nothing to be done for `all'.
>    CC    ppc64-softmmu/spapr_vscsi.o
> /home/anthony/git/qemu/hw/spapr_vscsi.c: In function 
> ‘vscsi_command_complete’:
> /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: ‘s’ undeclared 
> (first use in this function)
> /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: note: each undeclared 
> identifier is reported only once for each function it appears in
> 
> This file is only built when libfdt is installed which is probably
> why 
> you didn't catch it.
> 
> Ben/David, is there a way we can still build most of this stuff
> without 
> libfdt?  libfdt is still not commonly packaged by some distros.

That would be hard ... the DT stuff is pretty deeply involved. Might be
easier to try to fix the distro :-)

Which ones ?

Cheers,
Ben.
Benjamin Herrenschmidt - July 19, 2011, 1:24 p.m.
On Tue, 2011-07-19 at 15:06 +0200, Kevin Wolf wrote:
> Am 19.07.2011 14:43, schrieb Anthony Liguori:
> > On 07/19/2011 05:15 AM, Kevin Wolf wrote:
> >> From: Hannes Reinecke<hare@suse.de>
> >>
> >> 'tag' is just an abstraction to identify the command
> >> from the driver. So we should make that explicit by
> >> replacing 'tag' with a driver-defined pointer 'hba_private'.
> >> This saves the lookup for driver handling several commands
> >> in parallel.
> >> 'tag' is still being kept for tracing purposes.
> >>
> >> Signed-off-by: Hannes Reinecke<hare@suse.de>
> >> Acked-by: Paolo Bonzini<pbonzini@redhat.com>
> >> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
> >> ---
> >>   hw/esp.c          |    2 +-
> >>   hw/lsi53c895a.c   |   22 ++++++++--------------
> >>   hw/scsi-bus.c     |    9 ++++++---
> >>   hw/scsi-disk.c    |    4 ++--
> >>   hw/scsi-generic.c |    5 +++--
> >>   hw/scsi.h         |   10 +++++++---
> >>   hw/spapr_vscsi.c  |   29 +++++++++--------------------
> >>   hw/usb-msd.c      |    9 +--------
> >>   8 files changed, 37 insertions(+), 53 deletions(-)
> >>
> >> diff --git a/hw/esp.c b/hw/esp.c
> >> index aa50800..9ddd637 100644
> >> --- a/hw/esp.c
> >> +++ b/hw/esp.c
> >> @@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
> >>
> >>       DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
> >>       lun = busid&  7;
> >> -    s->current_req = scsi_req_new(s->current_dev, 0, lun);
> >> +    s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
> >>       datalen = scsi_req_enqueue(s->current_req, buf);
> >>       s->ti_size = datalen;
> >>       if (datalen != 0) {
> >> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> >> index 940b43a..69eec1d 100644
> >> --- a/hw/lsi53c895a.c
> >> +++ b/hw/lsi53c895a.c
> >> @@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
> >>   static void lsi_request_cancelled(SCSIRequest *req)
> >>   {
> >>       LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> >> -    lsi_request *p;
> >> +    lsi_request *p = req->hba_private;
> >>
> >>       if (s->current&&  req == s->current->req) {
> >>           scsi_req_unref(req);
> >> @@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
> >>           return;
> >>       }
> >>
> >> -    p = lsi_find_by_tag(s, req->tag);
> >>       if (p) {
> >>           QTAILQ_REMOVE(&s->queue, p, next);
> >>           scsi_req_unref(req);
> >> @@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
> >>
> >>   /* Record that data is available for a queued command.  Returns zero if
> >>      the device was reselected, nonzero if the IO is deferred.  */
> >> -static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
> >> +static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
> >>   {
> >> -    lsi_request *p;
> >> -
> >> -    p = lsi_find_by_tag(s, tag);
> >> -    if (!p) {
> >> -        BADF("IO with unknown tag %d\n", tag);
> >> -        return 1;
> >> -    }
> >> +    lsi_request *p = req->hba_private;
> >>
> >>       if (p->pending) {
> >> -        BADF("Multiple IO pending for tag %d\n", tag);
> >> +        BADF("Multiple IO pending for request %p\n", p);
> >>       }
> >>       p->pending = len;
> >>       /* Reselect if waiting for it, or if reselection triggers an IRQ
> >> @@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
> >>       LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> >>       int out;
> >>
> >> -    if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
> >> +    if (s->waiting == 1 || !s->current || req->hba_private != s->current ||
> >>           (lsi_irq_on_rsl(s)&&  !(s->scntl1&  LSI_SCNTL1_CON))) {
> >> -        if (lsi_queue_tag(s, req->tag, len)) {
> >> +        if (lsi_queue_req(s, req, len)) {
> >>               return;
> >>           }
> >>       }
> >> @@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
> >>       assert(s->current == NULL);
> >>       s->current = qemu_mallocz(sizeof(lsi_request));
> >>       s->current->tag = s->select_tag;
> >> -    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
> >> +    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
> >> +                                   s->current);
> >>
> >>       n = scsi_req_enqueue(s->current->req, buf);
> >>       if (n) {
> >> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> >> index ad6a730..8b1a412 100644
> >> --- a/hw/scsi-bus.c
> >> +++ b/hw/scsi-bus.c
> >> @@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
> >>       return res;
> >>   }
> >>
> >> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun)
> >> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> >> +                            uint32_t lun, void *hba_private)
> >>   {
> >>       SCSIRequest *req;
> >>
> >> @@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
> >>       req->dev = d;
> >>       req->tag = tag;
> >>       req->lun = lun;
> >> +    req->hba_private = hba_private;
> >>       req->status = -1;
> >>       trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
> >>       return req;
> >>   }
> >>
> >> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
> >> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >> +                          void *hba_private)
> >>   {
> >> -    return d->info->alloc_req(d, tag, lun);
> >> +    return d->info->alloc_req(d, tag, lun, hba_private);
> >>   }
> >>
> >>   uint8_t *scsi_req_get_buf(SCSIRequest *req)
> >> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> >> index a8c7372..c2a99fe 100644
> >> --- a/hw/scsi-disk.c
> >> +++ b/hw/scsi-disk.c
> >> @@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
> >>   static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
> >>
> >>   static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
> >> -        uint32_t lun)
> >> +                                     uint32_t lun, void *hba_private)
> >>   {
> >>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
> >>       SCSIRequest *req;
> >>       SCSIDiskReq *r;
> >>
> >> -    req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun);
> >> +    req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun, hba_private);
> >>       r = DO_UPCAST(SCSIDiskReq, req, req);
> >>       r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
> >>       return req;
> >> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> >> index 8e59c7e..90345a7 100644
> >> --- a/hw/scsi-generic.c
> >> +++ b/hw/scsi-generic.c
> >> @@ -96,11 +96,12 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
> >>       return size;
> >>   }
> >>
> >> -static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
> >> +static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >> +                                     void *hba_private)
> >>   {
> >>       SCSIRequest *req;
> >>
> >> -    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
> >> +    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun, hba_private);
> >>       return req;
> >>   }
> >>
> >> diff --git a/hw/scsi.h b/hw/scsi.h
> >> index c1dca35..6b15bbc 100644
> >> --- a/hw/scsi.h
> >> +++ b/hw/scsi.h
> >> @@ -43,6 +43,7 @@ struct SCSIRequest {
> >>       } cmd;
> >>       BlockDriverAIOCB  *aiocb;
> >>       bool enqueued;
> >> +    void *hba_private;
> >>       QTAILQ_ENTRY(SCSIRequest) next;
> >>   };
> >>
> >> @@ -67,7 +68,8 @@ struct SCSIDeviceInfo {
> >>       DeviceInfo qdev;
> >>       scsi_qdev_initfn init;
> >>       void (*destroy)(SCSIDevice *s);
> >> -    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
> >> +    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
> >> +                              void *hba_private);
> >>       void (*free_req)(SCSIRequest *req);
> >>       int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
> >>       void (*read_data)(SCSIRequest *req);
> >> @@ -138,8 +140,10 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
> >>   int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
> >>   int scsi_sense_valid(SCSISense sense);
> >>
> >> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
> >> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
> >> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> >> +                            uint32_t lun, void *hba_private);
> >> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >> +                          void *hba_private);
> >>   int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
> >>   void scsi_req_free(SCSIRequest *req);
> >>   SCSIRequest *scsi_req_ref(SCSIRequest *req);
> >> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> >> index 1c901ef..9e1cb2e 100644
> >> --- a/hw/spapr_vscsi.c
> >> +++ b/hw/spapr_vscsi.c
> >> @@ -121,7 +121,7 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
> >>       return NULL;
> >>   }
> >>
> >> -static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
> >> +static void vscsi_put_req(vscsi_req *req)
> >>   {
> >>       if (req->sreq != NULL) {
> >>           scsi_req_unref(req->sreq);
> > 
> > This breaks the build:
> > 
> > make[1]: Nothing to be done for `all'.
> >    CC    ppc64-softmmu/spapr_vscsi.o
> > /home/anthony/git/qemu/hw/spapr_vscsi.c: In function 
> > ‘vscsi_command_complete’:
> > /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: ‘s’ undeclared 
> > (first use in this function)
> > /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: note: each undeclared 
> > identifier is reported only once for each function it appears in
> 
> Adding Hannes to CC.
> 
> > This file is only built when libfdt is installed which is probably why 
> > you didn't catch it.
> 
> Yes, I did build all targets, but probably don't have most optional
> dependencies installed. I guess I should install some more libraries on
> that machine.
> 
> But as you said, there is no libfdt package in RHEL, so it will not be
> among them, and I'd like to avoid installing things from source.

Well, there isn't much we can do, those targets are going to require
libfdt, and if the device-tree stuff spreads on other architectures
(which is somewhat happening in the kernel now), expect that requirement
to grow beyond powerpc soon in qemu too :-)

I've spotted an effort to do a fedora package there:

https://bugzilla.redhat.com/show_bug.cgi?id=443882

I don't know much about distro packaging (I stay carefully away from
it), but maybe that could provide a basis for a RHEL one ?

Cheers,
Ben.
David Gibson - July 20, 2011, 5:49 a.m.
On Tue, Jul 19, 2011 at 11:20:22PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2011-07-19 at 07:43 -0500, Anthony Liguori wrote:
> > 
> > This breaks the build:
> > 
> > make[1]: Nothing to be done for `all'.
> >    CC    ppc64-softmmu/spapr_vscsi.o
> > /home/anthony/git/qemu/hw/spapr_vscsi.c: In function 
> > ‘vscsi_command_complete’:
> > /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: ‘s’ undeclared 
> > (first use in this function)
> > /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: note: each undeclared 
> > identifier is reported only once for each function it appears in
> > 
> > This file is only built when libfdt is installed which is probably
> > why 
> > you didn't catch it.
> > 
> > Ben/David, is there a way we can still build most of this stuff
> > without 
> > libfdt?  libfdt is still not commonly packaged by some distros.
> 
> That would be hard ... the DT stuff is pretty deeply involved. Might be
> easier to try to fix the distro :-)

Actually, I think what we should do is embed libfdt into qemu.  It's
small, it will remove a bunch of irritating config dependencies, and
it's designed to embeddable in this way.

Patch

diff --git a/hw/esp.c b/hw/esp.c
index aa50800..9ddd637 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@  static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
 
     DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
     lun = busid & 7;
-    s->current_req = scsi_req_new(s->current_dev, 0, lun);
+    s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
     datalen = scsi_req_enqueue(s->current_req, buf);
     s->ti_size = datalen;
     if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 940b43a..69eec1d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -661,7 +661,7 @@  static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
 static void lsi_request_cancelled(SCSIRequest *req)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
-    lsi_request *p;
+    lsi_request *p = req->hba_private;
 
     if (s->current && req == s->current->req) {
         scsi_req_unref(req);
@@ -670,7 +670,6 @@  static void lsi_request_cancelled(SCSIRequest *req)
         return;
     }
 
-    p = lsi_find_by_tag(s, req->tag);
     if (p) {
         QTAILQ_REMOVE(&s->queue, p, next);
         scsi_req_unref(req);
@@ -680,18 +679,12 @@  static void lsi_request_cancelled(SCSIRequest *req)
 
 /* Record that data is available for a queued command.  Returns zero if
    the device was reselected, nonzero if the IO is deferred.  */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
+static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
 {
-    lsi_request *p;
-
-    p = lsi_find_by_tag(s, tag);
-    if (!p) {
-        BADF("IO with unknown tag %d\n", tag);
-        return 1;
-    }
+    lsi_request *p = req->hba_private;
 
     if (p->pending) {
-        BADF("Multiple IO pending for tag %d\n", tag);
+        BADF("Multiple IO pending for request %p\n", p);
     }
     p->pending = len;
     /* Reselect if waiting for it, or if reselection triggers an IRQ
@@ -743,9 +736,9 @@  static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
-    if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
+    if (s->waiting == 1 || !s->current || req->hba_private != s->current ||
         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
-        if (lsi_queue_tag(s, req->tag, len)) {
+        if (lsi_queue_req(s, req, len)) {
             return;
         }
     }
@@ -789,7 +782,8 @@  static void lsi_do_command(LSIState *s)
     assert(s->current == NULL);
     s->current = qemu_mallocz(sizeof(lsi_request));
     s->current->tag = s->select_tag;
-    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
+    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
+                                   s->current);
 
     n = scsi_req_enqueue(s->current->req, buf);
     if (n) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ad6a730..8b1a412 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -131,7 +131,8 @@  int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
     return res;
 }
 
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+                            uint32_t lun, void *hba_private)
 {
     SCSIRequest *req;
 
@@ -141,14 +142,16 @@  SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     req->dev = d;
     req->tag = tag;
     req->lun = lun;
+    req->hba_private = hba_private;
     req->status = -1;
     trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
     return req;
 }
 
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+                          void *hba_private)
 {
-    return d->info->alloc_req(d, tag, lun);
+    return d->info->alloc_req(d, tag, lun, hba_private);
 }
 
 uint8_t *scsi_req_get_buf(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a8c7372..c2a99fe 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -81,13 +81,13 @@  static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
 static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
 
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
-        uint32_t lun)
+                                     uint32_t lun, void *hba_private)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
     SCSIRequest *req;
     SCSIDiskReq *r;
 
-    req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun);
+    req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun, hba_private);
     r = DO_UPCAST(SCSIDiskReq, req, req);
     r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
     return req;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 8e59c7e..90345a7 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -96,11 +96,12 @@  static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
     return size;
 }
 
-static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
+static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
+                                     void *hba_private)
 {
     SCSIRequest *req;
 
-    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
+    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun, hba_private);
     return req;
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index c1dca35..6b15bbc 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -43,6 +43,7 @@  struct SCSIRequest {
     } cmd;
     BlockDriverAIOCB  *aiocb;
     bool enqueued;
+    void *hba_private;
     QTAILQ_ENTRY(SCSIRequest) next;
 };
 
@@ -67,7 +68,8 @@  struct SCSIDeviceInfo {
     DeviceInfo qdev;
     scsi_qdev_initfn init;
     void (*destroy)(SCSIDevice *s);
-    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
+    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
+                              void *hba_private);
     void (*free_req)(SCSIRequest *req);
     int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
     void (*read_data)(SCSIRequest *req);
@@ -138,8 +140,10 @@  extern const struct SCSISense sense_code_LUN_FAILURE;
 int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
 int scsi_sense_valid(SCSISense sense);
 
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+                            uint32_t lun, void *hba_private);
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+                          void *hba_private);
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 1c901ef..9e1cb2e 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -121,7 +121,7 @@  static struct vscsi_req *vscsi_get_req(VSCSIState *s)
     return NULL;
 }
 
-static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
+static void vscsi_put_req(vscsi_req *req)
 {
     if (req->sreq != NULL) {
         scsi_req_unref(req->sreq);
@@ -130,15 +130,6 @@  static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
     req->active = 0;
 }
 
-static vscsi_req *vscsi_find_req(VSCSIState *s, SCSIRequest *req)
-{
-    uint32_t tag = req->tag;
-    if (tag >= VSCSI_REQ_LIMIT || !s->reqs[tag].active) {
-        return NULL;
-    }
-    return &s->reqs[tag];
-}
-
 static void vscsi_decode_id_lun(uint64_t srp_lun, int *id, int *lun)
 {
     /* XXX Figure that one out properly ! This is crackpot */
@@ -454,7 +445,7 @@  static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     if (n) {
         req->senselen = n;
         vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-        vscsi_put_req(s, req);
+        vscsi_put_req(req);
         return;
     }
 
@@ -483,7 +474,7 @@  static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, sreq);
+    vscsi_req *req = sreq->hba_private;
     uint8_t *buf;
     int rc = 0;
 
@@ -530,8 +521,7 @@  static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
 static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status)
 {
-    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, sreq);
+    vscsi_req *req = sreq->hba_private;
     int32_t res_in = 0, res_out = 0;
 
     dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x status=0x%x, req=%p\n",
@@ -563,15 +553,14 @@  static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status)
         }
     }
     vscsi_send_rsp(s, req, 0, res_in, res_out);
-    vscsi_put_req(s, req);
+    vscsi_put_req(req);
 }
 
 static void vscsi_request_cancelled(SCSIRequest *sreq)
 {
-    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, sreq);
+    vscsi_req *req = sreq->hba_private;
 
-    vscsi_put_req(s, req);
+    vscsi_put_req(req);
 }
 
 static void vscsi_process_login(VSCSIState *s, vscsi_req *req)
@@ -659,7 +648,7 @@  static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
     }
 
     req->lun = lun;
-    req->sreq = scsi_req_new(sdev, req->qtag, lun);
+    req->sreq = scsi_req_new(sdev, req->qtag, lun, req);
     n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
 
     dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
@@ -858,7 +847,7 @@  static void vscsi_got_payload(VSCSIState *s, vscsi_crq *crq)
     }
 
     if (done) {
-        vscsi_put_req(s, req);
+        vscsi_put_req(req);
     }
 }
 
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 86582cc..bfea096 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -216,10 +216,6 @@  static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
 
-    if (req->tag != s->tag) {
-        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
-    }
-
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = len;
     s->scsi_buf = scsi_req_get_buf(req);
@@ -241,9 +237,6 @@  static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
 
-    if (req->tag != s->tag) {
-        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
-    }
     DPRINTF("Command complete %d\n", status);
     s->residue = s->data_len;
     s->result = status != 0;
@@ -387,7 +380,7 @@  static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
                     s->tag, cbw.flags, cbw.cmd_len, s->data_len);
             s->residue = 0;
             s->scsi_len = 0;
-            s->req = scsi_req_new(s->scsi_dev, s->tag, 0);
+            s->req = scsi_req_new(s->scsi_dev, s->tag, 0, NULL);
             scsi_req_enqueue(s->req, cbw.cmd);
             /* ??? Should check that USB and SCSI data transfer
                directions match.  */