| Message ID | 1309534555-22178-3-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/01/2011 05:35 PM, Hannes Reinecke wrote: > '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> > --- > 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 6d3f5d2..aa87197 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. */ Acked-by: Paolo Bonzini <pbonzini@redhat.com> Have a nice weekend. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01.07.2011, at 17:35, Hannes Reinecke wrote:
> This patch adds an emulation for the LSI Megaraid SAS 8708EM2 HBA.
Have you tried to execute the current version of megasas and actually do something with it? I just booted up openSUSE 11.4 rescue from DVD with a megasas adapter that contained a raw file backed by tmpfs. Creating a partition worked fine, but when running mkfs.ext3 and mounting afterwards, the mount fails saying there is no ext3 on the disk.
Sounds like data corruption to me :). I know that this used to work a while back, so it might be a regression recently?
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/01/2011 06:42 PM, Alexander Graf wrote: > > On 01.07.2011, at 17:35, Hannes Reinecke wrote: > >> This patch adds an emulation for the LSI Megaraid SAS 8708EM2 HBA. > > Have you tried to execute the current version of megasas and actually > do something with it? I just booted up openSUSE 11.4 rescue from DVD > with a megasas adapter that contained a raw file backed by tmpfs. > Creating a partition worked fine, but when running mkfs.ext3 and > mounting afterwards, the mount fails saying there is no ext3 on the disk. > > Sounds like data corruption to me :). I know that this used to work > a while back, so it might be a regression recently? > It worked here, in the sense that I've booted up an existing openSUSE 11.4 installation. But I wouldn't be surprised if the degradation to use bounce-buffers has some flaws. My guess here is that we have problems when the transfersizes larger as the internal bounce buffer. (I probably should be putting in some more references to 'bounce buffers' here to alert people that using bounce buffers in SCSI is the best way of killing performance) (And no, I will not getting into another dog-fight with Paul B. here. Virtio can do without bounce buffers. AHCI can. So I fail to see why SCSI has to rely on bounce buffers.) But enough of this, Yeah, bugfixing is needed here. I see what I can do. Cheers, Hannes
On 07/03/2011 04:36 PM, Paolo Bonzini wrote: > On 07/02/2011 03:50 PM, Hannes Reinecke wrote: >> (And no, I will not getting into another dog-fight with Paul B. here. >> Virtio can do without bounce buffers. AHCI can. So I fail to see why >> SCSI has to rely on bounce buffers.) > > I agree, but I do see why a SCSI device might prefer to rely on > bounce buffers for non-I/O commands. This is why in my last RFC > series for vmw_pvscsi I let the device choose whether to force a > bounce buffer or get an external iovec from the HBA. > Yes, sure, for non-I/O commands it's perfectly okay. Most of which will be emulated anyway. It's bounce buffers for I/O which kills performance. But I seem to have missed your last RFC (I'm not reading qemu-devel on a regular basis ...). Care to send me a pointer to it? Cheers, Hannes
On 07/04/2011 08:13 AM, Hannes Reinecke wrote: > On 07/03/2011 04:36 PM, Paolo Bonzini wrote: >> On 07/02/2011 03:50 PM, Hannes Reinecke wrote: >>> (And no, I will not getting into another dog-fight with Paul B. here. >>> Virtio can do without bounce buffers. AHCI can. So I fail to see why >>> SCSI has to rely on bounce buffers.) >> >> I agree, but I do see why a SCSI device might prefer to rely on >> bounce buffers for non-I/O commands. This is why in my last RFC >> series for vmw_pvscsi I let the device choose whether to force a >> bounce buffer or get an external iovec from the HBA. >> > Yes, sure, for non-I/O commands it's perfectly okay. > Most of which will be emulated anyway. > It's bounce buffers for I/O which kills performance. > > But I seem to have missed your last RFC (I'm not reading qemu-devel on a > regular basis ...). > Care to send me a pointer to it? Sure, http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg00668.html Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/04/2011 08:34 AM, Paolo Bonzini wrote: > On 07/04/2011 08:13 AM, Hannes Reinecke wrote: >> On 07/03/2011 04:36 PM, Paolo Bonzini wrote: >>> On 07/02/2011 03:50 PM, Hannes Reinecke wrote: >>>> (And no, I will not getting into another dog-fight with Paul B. >>>> here. >>>> Virtio can do without bounce buffers. AHCI can. So I fail to see >>>> why >>>> SCSI has to rely on bounce buffers.) >>> >>> I agree, but I do see why a SCSI device might prefer to rely on >>> bounce buffers for non-I/O commands. This is why in my last RFC >>> series for vmw_pvscsi I let the device choose whether to force a >>> bounce buffer or get an external iovec from the HBA. >>> >> Yes, sure, for non-I/O commands it's perfectly okay. >> Most of which will be emulated anyway. >> It's bounce buffers for I/O which kills performance. >> >> But I seem to have missed your last RFC (I'm not reading >> qemu-devel on a >> regular basis ...). >> Care to send me a pointer to it? > > Sure, > http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg00668.html > Cool. Exactly what I need. FWIW, feel free to add my 'Acked-by' to it. Any chance of getting them included? Cheers, Hannes
On 07/02/2011 06:14 PM, Stefan Hajnoczi wrote: > On Fri, Jul 1, 2011 at 4:35 PM, Hannes Reinecke<hare@suse.de> wrote: >> +static void megasas_mmio_writel(void *opaque, target_phys_addr_t addr, >> + uint32_t val) >> +{ >> + MPTState *s = opaque; >> + target_phys_addr_t frame_addr; >> + uint32_t frame_count; >> + int i; >> + >> + DPRINTF_REG("writel mmio %lx: %x\n", (unsigned long)addr, val); >> + >> + switch (addr) { >> + case MFI_IDB: >> + if (val& MFI_FWINIT_ABORT) { >> + /* Abort all pending cmds */ >> + for (i = 0; i<= s->fw_cmds; i++) { >> + megasas_abort_command(&s->frames[i]); >> + } >> + } >> + if (val& MFI_FWINIT_READY) { >> + /* move to FW READY */ >> + megasas_soft_reset(s); >> + } >> + if (val& MFI_FWINIT_MFIMODE) { >> + /* discard MFIs */ >> + } >> + break; >> + case MFI_OMSK: >> + s->intr_mask = val; >> + if (!MEGASAS_INTR_ENABLED(s)) { >> + qemu_irq_lower(s->dev.irq[0]); >> + } >> + break; >> + case MFI_ODCR0: >> + /* Update reply queue pointer */ >> + DPRINTF_QUEUE("Update reply queue head %x busy %d\n", >> + s->reply_queue_index, s->busy); >> + stl_phys(s->producer_pa, s->reply_queue_index); >> + s->doorbell = 0; >> + qemu_irq_lower(s->dev.irq[0]); >> + break; >> + case MFI_IQPH: >> + s->frame_hi = val; >> + break; >> + case MFI_IQPL: >> + case MFI_IQP: >> + /* Received MFI frame address */ >> + frame_addr = (val& ~0xFF); >> + /* Add possible 64 bit offset */ >> + frame_addr |= (uint64_t)s->frame_hi; > > Is this missing<< 32 before ORing the high bits? > Yes, true. Linux doesn't use the high part, so I haven't seen it yet. Might explain the strange Win7 failures I've had :-) Fixed. >> +static int megasas_scsi_uninit(PCIDevice *d) >> +{ >> + MPTState *s = DO_UPCAST(MPTState, dev, d); >> + >> + cpu_unregister_io_memory(s->mmio_io_addr); > > Need to unregister io_addr and queue_addr. > Yeah, the unregister function is a bit of a stub currently. Fixed. >> + >> + return 0; >> +} >> + >> +static const struct SCSIBusOps megasas_scsi_ops = { >> + .transfer_data = megasas_xfer_complete, >> + .complete = megasas_command_complete, >> + .cancel = megasas_command_cancel, >> +}; >> + >> +static int megasas_scsi_init(PCIDevice *dev) >> +{ >> + MPTState *s = DO_UPCAST(MPTState, dev, dev); >> + uint8_t *pci_conf; >> + int i; >> + >> + pci_conf = s->dev.config; >> + >> + /* PCI Vendor ID (word) */ >> + pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_LSI_LOGIC); >> + /* PCI device ID (word) */ >> + pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_LSI_SAS1078); >> + /* PCI subsystem ID */ >> + pci_set_word(&pci_conf[PCI_SUBSYSTEM_VENDOR_ID], 0x1000); >> + pci_set_word(&pci_conf[PCI_SUBSYSTEM_ID], 0x1013); >> + /* PCI base class code */ >> + pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_RAID); > > > PCIDeviceInfo now has vendor_id, device_id, and other fields. These > values can be set in the megasas_info definition below. > Argl. Interface change again. Okay, will be doing so. >> + >> + /* PCI latency timer = 0 */ >> + pci_conf[0x0d] = 0; >> + /* Interrupt pin 1 */ >> + pci_conf[0x3d] = 0x01; >> + >> + s->mmio_io_addr = cpu_register_io_memory(megasas_mmio_readfn, >> + megasas_mmio_writefn, s, >> + DEVICE_NATIVE_ENDIAN); >> + s->io_addr = cpu_register_io_memory(megasas_io_readfn, >> + megasas_io_writefn, s, >> + DEVICE_NATIVE_ENDIAN); >> + s->queue_addr = cpu_register_io_memory(megasas_queue_readfn, >> + megasas_queue_writefn, s, >> + DEVICE_NATIVE_ENDIAN); > > Should these be little-endian? > Presumably. Haven't tested on big-endian emulations as of now. >> + pci_register_bar((struct PCIDevice *)s, 0, 0x40000, >> + PCI_BASE_ADDRESS_SPACE_MEMORY, megasas_mmio_mapfunc); >> + pci_register_bar((struct PCIDevice *)s, 2, 256, >> + PCI_BASE_ADDRESS_SPACE_IO, megasas_io_mapfunc); >> + pci_register_bar((struct PCIDevice *)s, 3, 0x40000, >> + PCI_BASE_ADDRESS_SPACE_MEMORY, megasas_queue_mapfunc); >> + if (s->fw_sge>= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) { >> + s->fw_sge = MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE; >> + } else if (s->fw_sge>= 128 - MFI_PASS_FRAME_SIZE) { >> + s->fw_sge = 128 - MFI_PASS_FRAME_SIZE; >> + } else { >> + s->fw_sge = 64 - MFI_PASS_FRAME_SIZE; >> + } >> + if (s->fw_cmds> MEGASAS_MAX_FRAMES) { >> + s->fw_cmds = MEGASAS_MAX_FRAMES; >> + } >> + if (s->raid_mode_str) { >> + if (!strcmp(s->raid_mode_str, "jbod")) { >> + s->is_jbod = 1; >> + } else { >> + s->is_jbod = 0; >> + } >> + } >> + DPRINTF("Using %d sges, %d cmds, %s mode\n", >> + s->fw_sge, s->fw_cmds, s->is_jbod ? "jbod" : "raid"); >> + s->fw_luns = (MFI_MAX_LD> MAX_SCSI_DEVS) ? >> + MAX_SCSI_DEVS : MFI_MAX_LD; >> + s->producer_pa = 0; >> + s->consumer_pa = 0; >> + for (i = 0; i< s->fw_cmds; i++) { >> + s->frames[i].index = i; >> + s->frames[i].context = -1; >> + s->frames[i].pa = 0; >> + s->frames[i].state = s; >> + } > > It is not clear to me that all register state is initialized here. > megasas_soft_reset() seems to touch fw_state and intr_mask but will > not be called until mmio_writel(MFI_IDB, MFI_FWINIT_READY). Are there > any missing fields that need to be initialized here? > I was under the impression that we'll be getting a reset after initialising the device, so this should've been taken care of. And most of the mmio writes are safe against accidental misuse. I'll be adding a check to MFI_ODCR0, as this indeed might cause an error when used uninitialized. Cheers, Hannes
On 07/04/2011 09:26 AM, Hannes Reinecke wrote: >> > Cool. > Exactly what I need. > > FWIW, feel free to add my 'Acked-by' to it. > > Any chance of getting them included? I'm not very tied to pvscsi; I just needed an HBA that is not a joke by modern standards :) to play with the SCSI layer. There may be hope that megasas will come before pvscsi or eliminate the need for it altogether. So, feel free to pick up patches 5-8 from that series. That said, note that scsi-generic does *not* support scatter/gather in that series, so you should still make sure the fallback paths work well. :) In pvscsi I added a qdev property to toggle scatter/gather, it was useful for benchmarking. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/hw/esp.c b/hw/esp.c index 6d3f5d2..aa87197 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. */
'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> --- 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(-)