[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] scsi: esp: remove handling of SATN/STOP
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-devel] [PATCH v2] scsi: esp: remove handling of SATN/STOP |
Date: |
Fri, 17 Jun 2016 14:13:13 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0 |
On 17/06/16 10:55, Paolo Bonzini wrote:
> 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 <address@hidden>
> ---
> 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;
>
Unforunately this causes regressions on a few of my SPARC32 images:
NextStep, Solaris 1.1.2, NetBSD and Debian Etch all hang whilst
enumerating the SCSI bus with this patch applied.
ATB,
Mark.