From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDqVH-0007p9-Ph for qemu-devel@nongnu.org; Fri, 17 Jun 2016 05:55:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDqVE-00036i-Kx for qemu-devel@nongnu.org; Fri, 17 Jun 2016 05:55:55 -0400 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:35152) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDqVE-00036c-AK for qemu-devel@nongnu.org; Fri, 17 Jun 2016 05:55:52 -0400 Received: by mail-wm0-x244.google.com with SMTP id k184so17378470wme.2 for ; Fri, 17 Jun 2016 02:55:51 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Fri, 17 Jun 2016 11:55:48 +0200 Message-Id: <1466157348-14488-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH v2] scsi: esp: remove handling of SATN/STOP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: amit.shah@redhat.com, =?UTF-8?q?Herv=C3=A9=20Poussineau?= , ppandit@redhat.com The implementation of SATN/STOP is completely busted. The idea would be that the next DMA read is for a SCSI message and after that the adapter would transition to the command phase. The recent fix to SATN/STOP broke migration, which is one more reason to drop SATN/STOP handling completely. It is only used in practice to send 3-byte messages (target number + tag type + tag number) for tagged command queuing on adapters that lack the SATN3 command, and we do not advertise support for TCQ. Signed-off-by: Paolo Bonzini --- hw/scsi/esp.c | 58 +++++++++------------------------------------------ include/hw/scsi/esp.h | 4 ---- 2 files changed, 10 insertions(+), 52 deletions(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index baa0a2c..d74f8d6 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -192,23 +192,6 @@ static void handle_s_without_atn(ESPState *s) } } -static void handle_satn_stop(ESPState *s) -{ - if (s->dma && !s->dma_enabled) { - s->dma_cb = handle_satn_stop; - return; - } - s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf)); - if (s->cmdlen) { - trace_esp_handle_satn_stop(s->cmdlen); - s->do_cmd = 1; - s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD; - s->rregs[ESP_RINTR] = INTR_BS | INTR_FC; - s->rregs[ESP_RSEQ] = SEQ_CD; - esp_raise_irq(s); - } -} - static void write_response(ESPState *s) { trace_esp_write_response(s->status); @@ -246,13 +229,6 @@ static void esp_do_dma(ESPState *s) int to_device; len = s->dma_left; - if (s->do_cmd) { - trace_esp_do_dma(s->cmdlen, len); - assert (s->cmdlen <= sizeof(s->cmdbuf) && - len <= sizeof(s->cmdbuf) - s->cmdlen); - s->dma_memory_read(s->dma_opaque, &s->cmdbuf[s->cmdlen], len); - return; - } if (s->async_len == 0) { /* Defer until data is available. */ return; @@ -316,7 +292,6 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len) { ESPState *s = req->hba_private; - assert(!s->do_cmd); trace_esp_transfer_data(s->dma_left, s->ti_size); s->async_len = len; s->async_buf = scsi_req_get_buf(req); @@ -346,9 +321,7 @@ static void handle_ti(ESPState *s) } s->dma_counter = dmalen; - if (s->do_cmd) - minlen = (dmalen < ESP_CMDBUF_SZ) ? dmalen : ESP_CMDBUF_SZ; - else if (s->ti_size < 0) + if (s->ti_size < 0) minlen = (dmalen < -s->ti_size) ? dmalen : -s->ti_size; else minlen = (dmalen < s->ti_size) ? dmalen : s->ti_size; @@ -358,13 +331,6 @@ static void handle_ti(ESPState *s) s->rregs[ESP_RSTAT] &= ~STAT_TC; esp_do_dma(s); } - if (s->do_cmd) { - trace_esp_handle_ti_cmd(s->cmdlen); - s->ti_size = 0; - s->cmdlen = 0; - s->do_cmd = 0; - do_cmd(s, s->cmdbuf); - } } void esp_hard_reset(ESPState *s) @@ -376,7 +342,6 @@ void esp_hard_reset(ESPState *s) s->ti_rptr = 0; s->ti_wptr = 0; s->dma = 0; - s->do_cmd = 0; s->dma_cb = NULL; s->rregs[ESP_CFG1] = 7; @@ -450,13 +415,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val) s->rregs[ESP_RSTAT] &= ~STAT_TC; break; case ESP_FIFO: - if (s->do_cmd) { - if (s->cmdlen < ESP_CMDBUF_SZ) { - s->cmdbuf[s->cmdlen++] = val & 0xff; - } else { - trace_esp_error_fifo_overrun(); - } - } else if (s->ti_wptr == TI_BUFSZ - 1) { + if (s->ti_wptr == TI_BUFSZ - 1) { trace_esp_error_fifo_overrun(); } else { s->ti_size++; @@ -534,8 +493,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val) break; case CMD_SELATNS: trace_esp_mem_writeb_cmd_selatns(val); - handle_satn_stop(s); - break; + goto unhandled; case CMD_ENSEL: trace_esp_mem_writeb_cmd_ensel(val); s->rregs[ESP_RINTR] = 0; @@ -546,6 +504,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val) esp_raise_irq(s); break; default: + unhandled: trace_esp_error_unhandled_command(val); break; } @@ -585,9 +544,12 @@ const VMStateDescription vmstate_esp = { VMSTATE_BUFFER(ti_buf, ESPState), VMSTATE_UINT32(status, ESPState), VMSTATE_UINT32(dma, ESPState), - VMSTATE_BUFFER(cmdbuf, ESPState), - VMSTATE_UINT32(cmdlen, ESPState), - VMSTATE_UINT32(do_cmd, ESPState), + /* Used to be cmdbuf, cmdlen, do_cmd, but the implementation + * of "Select with ATN and stop" was totally busted. + */ + VMSTATE_UNUSED(16), + VMSTATE_UNUSED(4), + VMSTATE_UNUSED(4), VMSTATE_UINT32(dma_left, ESPState), VMSTATE_END_OF_LIST() } diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h index d2c4886..61cb8b4 100644 --- a/include/hw/scsi/esp.h +++ b/include/hw/scsi/esp.h @@ -14,7 +14,6 @@ void esp_init(hwaddr espaddr, int it_shift, #define ESP_REGS 16 #define TI_BUFSZ 16 -#define ESP_CMDBUF_SZ 32 typedef struct ESPState ESPState; @@ -32,9 +31,6 @@ struct ESPState { SCSIBus bus; SCSIDevice *current_dev; SCSIRequest *current_req; - uint8_t cmdbuf[ESP_CMDBUF_SZ]; - uint32_t cmdlen; - uint32_t do_cmd; /* The amount of data left in the current DMA transfer. */ uint32_t dma_left; -- 2.5.5