From ad36d1a533da91d3448029b4da1113c5b880f25d Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 14 Nov 2005 13:56:37 -0500 Subject: [libata ahci] error handling fixes Needed to get ATAPI working. - dump hardware error bits, if hardware signals an error - only reset hardware during timeout if a command was active - call ata_qc_complete() with a fine-grained error mask. Needed so that atapi_qc_complete() can distinguish between device errors and other errors. --- drivers/scsi/ahci.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c index 4e96ec5f2ff9..c710a7def7a7 100644 --- a/drivers/scsi/ahci.c +++ b/drivers/scsi/ahci.c @@ -603,7 +603,12 @@ static void ahci_intr_error(struct ata_port *ap, u32 irq_stat) writel(tmp, port_mmio + PORT_CMD); readl(port_mmio + PORT_CMD); /* flush */ - printk(KERN_WARNING "ata%u: error occurred, port reset\n", ap->id); + printk(KERN_WARNING "ata%u: error occurred, port reset (%s%s%s%s)\n", + ap->id, + irq_stat & PORT_IRQ_TF_ERR ? "taskf " : "", + irq_stat & PORT_IRQ_HBUS_ERR ? "hbus " : "", + irq_stat & PORT_IRQ_HBUS_DATA_ERR ? "hbus_data " : "", + irq_stat & PORT_IRQ_IF_ERR ? "if " : ""); } static void ahci_eng_timeout(struct ata_port *ap) @@ -618,13 +623,13 @@ static void ahci_eng_timeout(struct ata_port *ap) spin_lock_irqsave(&host_set->lock, flags); - ahci_intr_error(ap, readl(port_mmio + PORT_IRQ_STAT)); - qc = ata_qc_from_tag(ap, ap->active_tag); if (!qc) { printk(KERN_ERR "ata%u: BUG: timeout without command\n", ap->id); } else { + ahci_intr_error(ap, readl(port_mmio + PORT_IRQ_STAT)); + /* hack alert! We cannot use the supplied completion * function from inside the ->eh_strategy_handler() thread. * libata is the only user of ->eh_strategy_handler() in @@ -659,9 +664,18 @@ static inline int ahci_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc) } if (status & PORT_IRQ_FATAL) { - ahci_intr_error(ap, status); + unsigned int err_mask; + if (status & PORT_IRQ_TF_ERR) + err_mask = AC_ERR_DEV; + else if (status & PORT_IRQ_IF_ERR) + err_mask = AC_ERR_ATA_BUS; + else + err_mask = AC_ERR_HOST_BUS; + + if (err_mask != AC_ERR_DEV) + ahci_intr_error(ap, status); if (qc) - ata_qc_complete(qc, AC_ERR_OTHER); + ata_qc_complete(qc, err_mask); } return 1; -- cgit v1.2.3-59-g8ed1b From e1410f2d951d45aee3bdbcc05ecedaaa9db276e5 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 14 Nov 2005 14:06:26 -0500 Subject: [libata] fix bugs in ATAPI padding DMA mapping code The ATAPI pad-to-next-32bit-boundary code modifies the scatterlist's length variable, sometimes to zero. x86-64 platform would oops if a zero-length scatterlist entry was asked to be mapped. Work around this by ensuring that we never DMA-map a zero length buffer or SG entry. --- drivers/scsi/libata-core.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index d81db3a3d4b9..1ccaf467d516 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -2409,7 +2409,8 @@ static void ata_sg_clean(struct ata_queued_cmd *qc) pad_buf = ap->pad + (qc->tag * ATA_DMA_PAD_SZ); if (qc->flags & ATA_QCFLAG_SG) { - dma_unmap_sg(ap->host_set->dev, sg, qc->n_elem, dir); + if (qc->n_elem) + dma_unmap_sg(ap->host_set->dev, sg, qc->n_elem, dir); /* restore last sg */ sg[qc->orig_n_elem - 1].length += qc->pad_len; if (pad_buf) { @@ -2419,8 +2420,10 @@ static void ata_sg_clean(struct ata_queued_cmd *qc) kunmap_atomic(psg->page, KM_IRQ0); } } else { - dma_unmap_single(ap->host_set->dev, sg_dma_address(&sg[0]), - sg_dma_len(&sg[0]), dir); + if (sg_dma_len(&sg[0]) > 0) + dma_unmap_single(ap->host_set->dev, + sg_dma_address(&sg[0]), sg_dma_len(&sg[0]), + dir); /* restore sg */ sg->length += qc->pad_len; if (pad_buf) @@ -2619,6 +2622,11 @@ static int ata_sg_setup_one(struct ata_queued_cmd *qc) sg->length, qc->pad_len); } + if (!sg->length) { + sg_dma_address(sg) = 0; + goto skip_map; + } + dma_address = dma_map_single(ap->host_set->dev, qc->buf_virt, sg->length, dir); if (dma_mapping_error(dma_address)) { @@ -2628,6 +2636,7 @@ static int ata_sg_setup_one(struct ata_queued_cmd *qc) } sg_dma_address(sg) = dma_address; +skip_map: sg_dma_len(sg) = sg->length; DPRINTK("mapped buffer of %d bytes for %s\n", sg_dma_len(sg), @@ -2655,7 +2664,7 @@ static int ata_sg_setup(struct ata_queued_cmd *qc) struct ata_port *ap = qc->ap; struct scatterlist *sg = qc->__sg; struct scatterlist *lsg = &sg[qc->n_elem - 1]; - int n_elem, dir; + int n_elem, pre_n_elem, dir, trim_sg = 0; VPRINTK("ENTER, ata%u\n", ap->id); assert(qc->flags & ATA_QCFLAG_SG); @@ -2689,13 +2698,24 @@ static int ata_sg_setup(struct ata_queued_cmd *qc) sg_dma_len(psg) = ATA_DMA_PAD_SZ; /* trim last sg */ lsg->length -= qc->pad_len; + if (lsg->length == 0) + trim_sg = 1; DPRINTK("padding done, sg[%d].length=%u pad_len=%u\n", qc->n_elem - 1, lsg->length, qc->pad_len); } + pre_n_elem = qc->n_elem; + if (trim_sg && pre_n_elem) + pre_n_elem--; + + if (!pre_n_elem) { + n_elem = 0; + goto skip_map; + } + dir = qc->dma_dir; - n_elem = dma_map_sg(ap->host_set->dev, sg, qc->n_elem, dir); + n_elem = dma_map_sg(ap->host_set->dev, sg, pre_n_elem, dir); if (n_elem < 1) { /* restore last sg */ lsg->length += qc->pad_len; @@ -2704,6 +2724,7 @@ static int ata_sg_setup(struct ata_queued_cmd *qc) DPRINTK("%d sg elements mapped\n", n_elem); +skip_map: qc->n_elem = n_elem; return 0; -- cgit v1.2.3-59-g8ed1b From 2c13b7cee045af689b36349c2bc6a9ed6e3d73fa Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 14 Nov 2005 14:14:16 -0500 Subject: [libata] minor fixes, new helpers - in ata_dev_identify(), don't assume that all devices are either ATA or ATAPI. In the future, this code will see port multipliers and other devices. - make a debugging printk less verbose - add new helper ata_qc_reinit() - add new helper BPRINTK() and port flag ATA_FLAG_DEBUGMSG, for fine-grained debugging use. --- drivers/scsi/libata-core.c | 11 +++-------- include/linux/libata.h | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index 1ccaf467d516..3855bfa8e8d4 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -1263,7 +1263,7 @@ retry: } /* ATAPI-specific feature tests */ - else { + else if (dev->class == ATA_DEV_ATAPI) { if (ata_id_is_ata(dev->id)) /* sanity check */ goto err_out_nosup; @@ -2399,7 +2399,7 @@ static void ata_sg_clean(struct ata_queued_cmd *qc) if (qc->flags & ATA_QCFLAG_SINGLE) assert(qc->n_elem == 1); - DPRINTK("unmapping %u sg elements\n", qc->n_elem); + VPRINTK("unmapping %u sg elements\n", qc->n_elem); /* if we padded the buffer out to 32-bit bound, and data * xfer direction is from-device, we must copy from the @@ -3432,16 +3432,11 @@ struct ata_queued_cmd *ata_qc_new_init(struct ata_port *ap, qc = ata_qc_new(ap); if (qc) { - qc->__sg = NULL; - qc->flags = 0; qc->scsicmd = NULL; qc->ap = ap; qc->dev = dev; - qc->cursect = qc->cursg = qc->cursg_ofs = 0; - qc->nsect = 0; - qc->nbytes = qc->curbytes = 0; - ata_tf_init(ap, &qc->tf, dev->devno); + ata_qc_reinit(qc); } return qc; diff --git a/include/linux/libata.h b/include/linux/libata.h index ad5996183ec2..f2dbb684ce9e 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -59,6 +59,8 @@ #define VPRINTK(fmt, args...) #endif /* ATA_DEBUG */ +#define BPRINTK(fmt, args...) if (ap->flags & ATA_FLAG_DEBUGMSG) printk(KERN_ERR "%s: " fmt, __FUNCTION__, ## args) + #ifdef ATA_NDEBUG #define assert(expr) #else @@ -119,6 +121,7 @@ enum { ATA_FLAG_PIO_DMA = (1 << 8), /* PIO cmds via DMA */ ATA_FLAG_NOINTR = (1 << 9), /* FIXME: Remove this once * proper HSM is in place. */ + ATA_FLAG_DEBUGMSG = (1 << 10), ATA_QCFLAG_ACTIVE = (1 << 1), /* cmd not yet ack'd to scsi lyer */ ATA_QCFLAG_SG = (1 << 3), /* have s/g table? */ @@ -659,6 +662,17 @@ static inline void ata_tf_init(struct ata_port *ap, struct ata_taskfile *tf, uns tf->device = ATA_DEVICE_OBS | ATA_DEV1; } +static inline void ata_qc_reinit(struct ata_queued_cmd *qc) +{ + qc->__sg = NULL; + qc->flags = 0; + qc->cursect = qc->cursg = qc->cursg_ofs = 0; + qc->nsect = 0; + qc->nbytes = qc->curbytes = 0; + + ata_tf_init(qc->ap, &qc->tf, qc->dev->devno); +} + /** * ata_irq_on - Enable interrupts on a port. -- cgit v1.2.3-59-g8ed1b From c6e6e666cbfe40f0d7fb1a293ff6332973acac37 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 14 Nov 2005 14:50:05 -0500 Subject: [libata] REQUEST SENSE handling fixes - Move ATAPI check-condition handling out of the timeout handler - Use multi-qc-issue feature to issue REQUEST SENSE ATAPI PACKET command upon receiving an ATAPI check-condition. This cleans things up a lot, and eliminates a nasty recursion bug. --- drivers/scsi/libata-core.c | 22 ------------- drivers/scsi/libata-scsi.c | 77 ++++++++++++++++++++++++++-------------------- drivers/scsi/libata.h | 2 -- 3 files changed, 43 insertions(+), 58 deletions(-) diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index 3855bfa8e8d4..ba1eb8b38e00 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -3284,32 +3284,11 @@ static void ata_qc_timeout(struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; struct ata_host_set *host_set = ap->host_set; - struct ata_device *dev = qc->dev; u8 host_stat = 0, drv_stat; unsigned long flags; DPRINTK("ENTER\n"); - /* FIXME: doesn't this conflict with timeout handling? */ - if (qc->dev->class == ATA_DEV_ATAPI && qc->scsicmd) { - struct scsi_cmnd *cmd = qc->scsicmd; - - if (!(cmd->eh_eflags & SCSI_EH_CANCEL_CMD)) { - - /* finish completing original command */ - spin_lock_irqsave(&host_set->lock, flags); - __ata_qc_complete(qc); - spin_unlock_irqrestore(&host_set->lock, flags); - - atapi_request_sense(ap, dev, cmd); - - cmd->result = (CHECK_CONDITION << 1) | (DID_OK << 16); - scsi_finish_command(cmd); - - goto out; - } - } - spin_lock_irqsave(&host_set->lock, flags); /* hack alert! We cannot use the supplied completion @@ -3348,7 +3327,6 @@ static void ata_qc_timeout(struct ata_queued_cmd *qc) spin_unlock_irqrestore(&host_set->lock, flags); -out: DPRINTK("EXIT\n"); } diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c index 0df4b682965d..3b4ca55a3332 100644 --- a/drivers/scsi/libata-scsi.c +++ b/drivers/scsi/libata-scsi.c @@ -1955,22 +1955,44 @@ void ata_scsi_badcmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *), u8 done(cmd); } -void atapi_request_sense(struct ata_port *ap, struct ata_device *dev, - struct scsi_cmnd *cmd) +static int atapi_sense_complete(struct ata_queued_cmd *qc,unsigned int err_mask) { - DECLARE_COMPLETION(wait); - struct ata_queued_cmd *qc; - unsigned long flags; - int rc; + if (err_mask && ((err_mask & AC_ERR_DEV) == 0)) + /* FIXME: not quite right; we don't want the + * translation of taskfile registers into + * a sense descriptors, since that's only + * correct for ATA, not ATAPI + */ + ata_gen_ata_desc_sense(qc); - DPRINTK("ATAPI request sense\n"); + qc->scsidone(qc->scsicmd); + return 0; +} - qc = ata_qc_new_init(ap, dev); - BUG_ON(qc == NULL); +/* is it pointless to prefer PIO for "safety reasons"? */ +static inline int ata_pio_use_silly(struct ata_port *ap) +{ + return (ap->flags & ATA_FLAG_PIO_DMA); +} + +static void atapi_request_sense(struct ata_queued_cmd *qc) +{ + struct ata_port *ap = qc->ap; + struct scsi_cmnd *cmd = qc->scsicmd; + + DPRINTK("ATAPI request sense\n"); /* FIXME: is this needed? */ memset(cmd->sense_buffer, 0, sizeof(cmd->sense_buffer)); + ap->ops->tf_read(ap, &qc->tf); + + /* fill these in, for the case where they are -not- overwritten */ + cmd->sense_buffer[0] = 0x70; + cmd->sense_buffer[2] = qc->tf.feature >> 4; + + ata_qc_reinit(qc); + ata_sg_init_one(qc, cmd->sense_buffer, sizeof(cmd->sense_buffer)); qc->dma_dir = DMA_FROM_DEVICE; @@ -1981,22 +2003,20 @@ void atapi_request_sense(struct ata_port *ap, struct ata_device *dev, qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; qc->tf.command = ATA_CMD_PACKET; - qc->tf.protocol = ATA_PROT_ATAPI; - qc->tf.lbam = (8 * 1024) & 0xff; - qc->tf.lbah = (8 * 1024) >> 8; + if (ata_pio_use_silly(ap)) { + qc->tf.protocol = ATA_PROT_ATAPI_DMA; + qc->tf.feature |= ATAPI_PKT_DMA; + } else { + qc->tf.protocol = ATA_PROT_ATAPI; + qc->tf.lbam = (8 * 1024) & 0xff; + qc->tf.lbah = (8 * 1024) >> 8; + } qc->nbytes = SCSI_SENSE_BUFFERSIZE; - qc->waiting = &wait; - qc->complete_fn = ata_qc_complete_noop; + qc->complete_fn = atapi_sense_complete; - spin_lock_irqsave(&ap->host_set->lock, flags); - rc = ata_qc_issue(qc); - spin_unlock_irqrestore(&ap->host_set->lock, flags); - - if (rc) - ata_port_disable(ap); - else - wait_for_completion(&wait); + if (ata_qc_issue(qc)) + ata_qc_complete(qc, AC_ERR_OTHER); DPRINTK("EXIT\n"); } @@ -2008,19 +2028,8 @@ static int atapi_qc_complete(struct ata_queued_cmd *qc, unsigned int err_mask) VPRINTK("ENTER, err_mask 0x%X\n", err_mask); if (unlikely(err_mask & AC_ERR_DEV)) { - DPRINTK("request check condition\n"); - - /* FIXME: command completion with check condition - * but no sense causes the error handler to run, - * which then issues REQUEST SENSE, fills in the sense - * buffer, and completes the command (for the second - * time). We need to issue REQUEST SENSE some other - * way, to avoid completing the command twice. - */ cmd->result = SAM_STAT_CHECK_CONDITION; - - qc->scsidone(cmd); - + atapi_request_sense(qc); return 1; } diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h index fad051ca4672..74a84e0ec0a4 100644 --- a/drivers/scsi/libata.h +++ b/drivers/scsi/libata.h @@ -54,8 +54,6 @@ extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg); /* libata-scsi.c */ -extern void atapi_request_sense(struct ata_port *ap, struct ata_device *dev, - struct scsi_cmnd *cmd); extern void ata_scsi_scan_host(struct ata_port *ap); extern int ata_scsi_error(struct Scsi_Host *host); extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf, -- cgit v1.2.3-59-g8ed1b From 9f68a24853dd37d18f2cfa979cf4e131f5a10e8f Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 15 Nov 2005 14:03:47 -0500 Subject: [libata ahci] command completion fixes, improved debug msgs - Fix a regression in command completion, which prevented the restart of the DMA engine after the device throws an error. - Pack more hardware info into the port-reset error message. - Promote "welcome to our timeout" message from debug msg to normal printk. --- drivers/scsi/ahci.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c index c710a7def7a7..894e7113e0b3 100644 --- a/drivers/scsi/ahci.c +++ b/drivers/scsi/ahci.c @@ -565,6 +565,17 @@ static void ahci_intr_error(struct ata_port *ap, u32 irq_stat) u32 tmp; int work; + printk(KERN_WARNING "ata%u: port reset, " + "p_is %x is %x pis %x cmd %x tf %x ss %x se %x\n", + ap->id, + irq_stat, + readl(mmio + HOST_IRQ_STAT), + readl(port_mmio + PORT_IRQ_STAT), + readl(port_mmio + PORT_CMD), + readl(port_mmio + PORT_TFDATA), + readl(port_mmio + PORT_SCR_STAT), + readl(port_mmio + PORT_SCR_ERR)); + /* stop DMA */ tmp = readl(port_mmio + PORT_CMD); tmp &= ~PORT_CMD_START; @@ -602,13 +613,6 @@ static void ahci_intr_error(struct ata_port *ap, u32 irq_stat) tmp |= PORT_CMD_START; writel(tmp, port_mmio + PORT_CMD); readl(port_mmio + PORT_CMD); /* flush */ - - printk(KERN_WARNING "ata%u: error occurred, port reset (%s%s%s%s)\n", - ap->id, - irq_stat & PORT_IRQ_TF_ERR ? "taskf " : "", - irq_stat & PORT_IRQ_HBUS_ERR ? "hbus " : "", - irq_stat & PORT_IRQ_HBUS_DATA_ERR ? "hbus_data " : "", - irq_stat & PORT_IRQ_IF_ERR ? "if " : ""); } static void ahci_eng_timeout(struct ata_port *ap) @@ -619,7 +623,7 @@ static void ahci_eng_timeout(struct ata_port *ap) struct ata_queued_cmd *qc; unsigned long flags; - DPRINTK("ENTER\n"); + printk(KERN_WARNING "ata%u: handling error/timeout\n", ap->id); spin_lock_irqsave(&host_set->lock, flags); @@ -672,8 +676,9 @@ static inline int ahci_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc) else err_mask = AC_ERR_HOST_BUS; - if (err_mask != AC_ERR_DEV) - ahci_intr_error(ap, status); + /* command processing has stopped due to error; restart */ + ahci_intr_error(ap, status); + if (qc) ata_qc_complete(qc, err_mask); } -- cgit v1.2.3-59-g8ed1b