We're still seeing a lot of issues with NCQ implementation in drive firmwares. Sprious FISes during NCQ command phase occur on many drives and some of them seem potentially dangerous (at least to me). Until we find the solution, spurious messages can give us more info. Improve and limit them such that more info can be reported while not disturbing users too much.
Unified · Split
| @@ -75,6 +75,7 @@ enum { · 74 unmodified lines | ||
| 75 | 75 | AHCI_CMD_CLR_BUSY = (1 << 10), |
| 76 | 76 | |
| 77 | 77 | RX_FIS_D2H_REG = 0x40, /* offset of D2H Register FIS data */ |
| 78 | + RX_FIS_SDB = 0x58, /* offset of SDB FIS data */ | |
| 78 | 79 | RX_FIS_UNK = 0x60, /* offset of Unknown FIS data */ |
| 79 | 80 | |
| 80 | 81 | board_ahci = 0, |
| @@ -202,6 +203,10 @@ struct ahci_port_priv { · 121 unmodified lines | ||
| 202 | 203 | dma_addr_t cmd_tbl_dma; |
| 203 | 204 | void *rx_fis; |
| 204 | 205 | dma_addr_t rx_fis_dma; |
| 206 | + /* for NCQ spurious interrupt analysis */ | |
| 207 | + int ncq_saw_spurious_sdb_cnt; | |
| 208 | + unsigned int ncq_saw_d2h:1; | |
| 209 | + unsigned int ncq_saw_dmas:1; | |
| 205 | 210 | }; |
| 206 | 211 | |
| 207 | 212 | static u32 ahci_scr_read (struct ata_port *ap, unsigned int sc_reg); |
| @@ -1109,8 +1114,9 @@ static void ahci_host_intr(struct ata_port *ap) · 901 unmodified lines | ||
| 1109 | 1114 | void __iomem *mmio = ap->host->mmio_base; |
| 1110 | 1115 | void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no); |
| 1111 | 1116 | struct ata_eh_info *ehi = &ap->eh_info; |
| 1117 | + struct ahci_port_priv *pp = ap->private_data; | |
| 1112 | 1118 | u32 status, qc_active; |
| 1113 | − int rc; | |
| 1119 | + int rc, known_irq = 0; | |
| 1114 | 1120 | |
| 1115 | 1121 | status = readl(port_mmio + PORT_IRQ_STAT); |
| 1116 | 1122 | writel(status, port_mmio + PORT_IRQ_STAT); |
| @@ -1137,17 +1143,52 @@ static void ahci_host_intr(struct ata_port *ap) · 20 unmodified lines | ||
| 1137 | 1143 | |
| 1138 | 1144 | /* hmmm... a spurious interupt */ |
| 1139 | 1145 | |
| 1140 | − /* some devices send D2H reg with I bit set during NCQ command phase */ | |
| 1141 | − if (ap->sactive && (status & PORT_IRQ_D2H_REG_FIS)) | |
| 1146 | + /* if !NCQ, ignore. No modern ATA device has broken HSM | |
| 1147 | + * implementation for non-NCQ commands. | |
| 1148 | + */ | |
| 1149 | + if (!ap->sactive) | |
| 1142 | 1150 | return; |
| 1143 | 1151 | |
| 1144 | − /* ignore interim PIO setup fis interrupts */ | |
| 1145 | − if (ata_tag_valid(ap->active_tag) && (status & PORT_IRQ_PIOS_FIS)) | |
| 1146 | − return; | |
| 1152 | + if (status & PORT_IRQ_D2H_REG_FIS) { | |
| 1153 | + if (!pp->ncq_saw_d2h) | |
| 1154 | + ata_port_printk(ap, KERN_INFO, | |
| 1155 | + "D2H reg with I during NCQ, " | |
| 1156 | + "this message won't be printed again\n"); | |
| 1157 | + pp->ncq_saw_d2h = 1; | |
| 1158 | + known_irq = 1; | |
| 1159 | + } | |
| 1160 | + | |
| 1161 | + if (status & PORT_IRQ_DMAS_FIS) { | |
| 1162 | + if (!pp->ncq_saw_dmas) | |
| 1163 | + ata_port_printk(ap, KERN_INFO, | |
| 1164 | + "DMAS FIS during NCQ, " | |
| 1165 | + "this message won't be printed again\n"); | |
| 1166 | + pp->ncq_saw_dmas = 1; | |
| 1167 | + known_irq = 1; | |
| 1168 | + } | |
| 1169 | + | |
| 1170 | + if (status & PORT_IRQ_SDB_FIS && | |
| 1171 | + pp->ncq_saw_spurious_sdb_cnt < 10) { | |
| 1172 | + /* SDB FIS containing spurious completions might be | |
| 1173 | + * dangerous, we need to know more about them. Print | |
| 1174 | + * more of it. | |
| 1175 | + */ | |
| 1176 | + const u32 *f = pp->rx_fis + RX_FIS_SDB; | |
| 1177 | + | |
| 1178 | + ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ " | |
| 1179 | + "issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n", | |
| 1180 | + readl(port_mmio + PORT_CMD_ISSUE), | |
| 1181 | + readl(port_mmio + PORT_SCR_ACT), f[0], f[1], | |
| 1182 | + pp->ncq_saw_spurious_sdb_cnt < 10 ? | |
| 1183 | + "" : ", shutting up"); | |
| 1184 | + | |
| 1185 | + pp->ncq_saw_spurious_sdb_cnt++; | |
| 1186 | + known_irq = 1; | |
| 1187 | + } | |
| 1147 | 1188 | |
| 1148 | − if (ata_ratelimit()) | |
| 1189 | + if (!known_irq) | |
| 1149 | 1190 | ata_port_printk(ap, KERN_INFO, "spurious interrupt " |
| 1150 | − "(irq_stat 0x%x active_tag %d sactive 0x%x)\n", | |
| 1191 | + "(irq_stat 0x%x active_tag 0x%x sactive 0x%x)\n", | |
| 1151 | 1192 | status, ap->active_tag, ap->sactive); |
| 1152 | 1193 | } |
| 1153 | 1194 | |