From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjAVX-0003zq-08 for qemu-devel@nongnu.org; Tue, 19 Jul 2011 09:38:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QjAVS-0003hE-BN for qemu-devel@nongnu.org; Tue, 19 Jul 2011 09:38:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32530) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjAVR-0003h0-IL for qemu-devel@nongnu.org; Tue, 19 Jul 2011 09:38:38 -0400 Message-ID: <4E258984.7020901@redhat.com> Date: Tue, 19 Jul 2011 15:41:24 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311070524-13533-1-git-send-email-kwolf@redhat.com> <1311070524-13533-6-git-send-email-kwolf@redhat.com> <4E257BE6.7080108@codemonkey.ws> <4E25816E.906@redhat.com> <4E2585F6.7090909@suse.de> In-Reply-To: <4E2585F6.7090909@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 05/21] scsi: Add 'hba_private' to SCSIRequest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke Cc: qemu-devel@nongnu.org, David Gibson Am 19.07.2011 15:26, schrieb Hannes Reinecke: > On 07/19/2011 03:06 PM, 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 >>>> >>>> '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 >>>> Acked-by: Paolo Bonzini >>>> Signed-off-by: Kevin Wolf >>>> --- >>>> 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 *b= uf, uint8_t busid) >>>> >>>> DPRINTF("do_busid_cmd: busid 0x%x\n", busid); >>>> lun =3D busid& 7; >>>> - s->current_req =3D scsi_req_new(s->current_dev, 0, lun); >>>> + s->current_req =3D scsi_req_new(s->current_dev, 0, lun, NULL); >>>> datalen =3D scsi_req_enqueue(s->current_req, buf); >>>> s->ti_size =3D datalen; >>>> if (datalen !=3D 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 =3D DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.= parent); >>>> - lsi_request *p; >>>> + lsi_request *p =3D req->hba_private; >>>> >>>> if (s->current&& req =3D=3D s->current->req) { >>>> scsi_req_unref(req); >>>> @@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *r= eq) >>>> return; >>>> } >>>> >>>> - p =3D 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 z= ero 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 le= n) >>>> { >>>> - lsi_request *p; >>>> - >>>> - p =3D lsi_find_by_tag(s, tag); >>>> - if (!p) { >>>> - BADF("IO with unknown tag %d\n", tag); >>>> - return 1; >>>> - } >>>> + lsi_request *p =3D 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 =3D 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 =3D DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.= parent); >>>> int out; >>>> >>>> - if (s->waiting =3D=3D 1 || !s->current || req->tag !=3D s->curr= ent->tag || >>>> + if (s->waiting =3D=3D 1 || !s->current || req->hba_private !=3D= 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 =3D=3D NULL); >>>> s->current =3D qemu_mallocz(sizeof(lsi_request)); >>>> s->current->tag =3D s->select_tag; >>>> - s->current->req =3D scsi_req_new(dev, s->current->tag, s->curre= nt_lun); >>>> + s->current->req =3D scsi_req_new(dev, s->current->tag, s->curre= nt_lun, >>>> + s->current); >>>> >>>> n =3D 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 ta= g, uint32_t lun) >>>> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t ta= g, >>>> + uint32_t lun, void *hba_private) >>>> { >>>> SCSIRequest *req; >>>> >>>> @@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSID= evice *d, uint32_t tag, uint32_t l >>>> req->dev =3D d; >>>> req->tag =3D tag; >>>> req->lun =3D lun; >>>> + req->hba_private =3D hba_private; >>>> req->status =3D -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 *out= buf); >>>> >>>> static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, >>>> - uint32_t lun) >>>> + uint32_t lun, void *hba_privat= e) >>>> { >>>> SCSIDiskState *s =3D DO_UPCAST(SCSIDiskState, qdev, d); >>>> SCSIRequest *req; >>>> SCSIDiskReq *r; >>>> >>>> - req =3D scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun); >>>> + req =3D scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun, = hba_private); >>>> r =3D DO_UPCAST(SCSIDiskReq, req, req); >>>> r->iov.iov_base =3D 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, uint= 8_t *outbuf, int len) >>>> return size; >>>> } >>>> >>>> -static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, u= int32_t lun) >>>> +static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, u= int32_t lun, >>>> + void *hba_private) >>>> { >>>> SCSIRequest *req; >>>> >>>> - req =3D scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun); >>>> + req =3D 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_FA= ILURE; >>>> 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 ta= g, 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 ta= g, >>>> + 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(VSCSIStat= e *s) >>>> return NULL; >>>> } >>>> >>>> -static void vscsi_put_req(VSCSIState *s, vscsi_req *req) >>>> +static void vscsi_put_req(vscsi_req *req) >>>> { >>>> if (req->sreq !=3D 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 >>> =91vscsi_command_complete=92: >>> /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: =91s=92 undecl= ared >>> (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 wh= y >>> 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 o= n >> 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. >> > Fix is attached. >=20 > How to proceed? Do you require a new patch or can you fix it up=20 > yourself? This is good enough for me, I'll update the commit. Kevin