Re: [PATCH v15 02/11] esp: move get_cmd() post-DMA code to get_cmd_cb()

From: Paolo Bonzini <pbonzini@redhat.com>
To: Laurent Vivier <laurent@vivier.eu>, qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>, "Fam Zheng" <fam@euphon.net>,
	qemu-block@nongnu.org, "Thomas Huth" <huth@tuxfamily.org>,
	"Jason Wang" <jasowang@redhat.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>,
	"Hervé Poussineau" <hpoussin@reactos.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>
Subject: Re: [PATCH v15 02/11] esp: move get_cmd() post-DMA code to get_cmd_cb()
Date: Sun, 27 Oct 2019 18:02:36 +0100	[thread overview]
Message-ID: <c5bf404d-a650-8ae5-aeab-9d0c07aea2ae@redhat.com> (raw)
In-Reply-To: <20191026164546.30020-3-laurent@vivier.eu>

On 26/10/19 18:45, Laurent Vivier wrote:
> This will be needed to implement pseudo-DMA
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/scsi/esp.c | 46 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 09b28cba17..0230ede21d 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -84,6 +84,34 @@ void esp_request_cancelled(SCSIRequest *req)
>      }
>  }
>  
> +static int get_cmd_cb(ESPState *s)
> +{
> +    int target;
> +
> +    target = s->wregs[ESP_WBUSID] & BUSID_DID;
> +
> +    s->ti_size = 0;
> +    s->ti_rptr = 0;
> +    s->ti_wptr = 0;
> +
> +    if (s->current_req) {
> +        /* Started a new command before the old one finished.  Cancel it.  */
> +        scsi_req_cancel(s->current_req);
> +        s->async_len = 0;
> +    }
> +
> +    s->current_dev = scsi_device_find(&s->bus, 0, target, 0);
> +    if (!s->current_dev) {
> +        /* No such drive */
> +        s->rregs[ESP_RSTAT] = 0;
> +        s->rregs[ESP_RINTR] = INTR_DC;
> +        s->rregs[ESP_RSEQ] = SEQ_0;
> +        esp_raise_irq(s);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
>  {
>      uint32_t dmalen;
> @@ -108,23 +136,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
>      }
>      trace_esp_get_cmd(dmalen, target);
>  
> -    s->ti_size = 0;
> -    s->ti_rptr = 0;
> -    s->ti_wptr = 0;
> -
> -    if (s->current_req) {
> -        /* Started a new command before the old one finished.  Cancel it.  */
> -        scsi_req_cancel(s->current_req);
> -        s->async_len = 0;
> -    }
> -
> -    s->current_dev = scsi_device_find(&s->bus, 0, target, 0);
> -    if (!s->current_dev) {
> -        // No such drive
> -        s->rregs[ESP_RSTAT] = 0;
> -        s->rregs[ESP_RINTR] = INTR_DC;
> -        s->rregs[ESP_RSEQ] = SEQ_0;
> -        esp_raise_irq(s);
> +    if (get_cmd_cb(s) < 0) {
>          return 0;
>      }
>      return dmalen;
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>