On Wed, 5 Jun 2019, Ming Lei wrote:
> The driver expects that SCSI SGL is a simply array of SG, and itereate
> the SGL via integer index. This way is obviously wrong, because of
> SG_CHAIN.
>
> Fixes it by using sgl helper.
>
> V2:
> - as suggested by Finn Thain, introduce .prv_sg for getting previous
> scatterlist pointer
>
Thanks. I'd forgotten about the consequences for SAVE/RESTORE POINTERS
until I saw your patch.
I can't test the change to the IGNORE WIDE RESIDUE message handling but I
did confirm that the patch series works in QEMU/m68k.
Reviewed-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
--
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> Cc: Ewan D. Milne <emilne@xxxxxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxxx>
> Cc: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/scsi/esp_scsi.c | 20 +++++++++++++-------
> drivers/scsi/esp_scsi.h | 2 ++
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 76e7ca864d6a..bb88995a12c7 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -371,6 +371,7 @@ static void esp_map_dma(struct esp *esp, struct scsi_cmnd *cmd)
> struct esp_cmd_priv *spriv = ESP_CMD_PRIV(cmd);
> struct scatterlist *sg = scsi_sglist(cmd);
> int total = 0, i;
> + struct scatterlist *s;
>
> if (cmd->sc_data_direction == DMA_NONE)
> return;
> @@ -381,16 +382,18 @@ static void esp_map_dma(struct esp *esp, struct scsi_cmnd *cmd)
> * a dma address, so perform an identity mapping.
> */
> spriv->num_sg = scsi_sg_count(cmd);
> - for (i = 0; i < spriv->num_sg; i++) {
> - sg[i].dma_address = (uintptr_t)sg_virt(&sg[i]);
> - total += sg_dma_len(&sg[i]);
> +
> + scsi_for_each_sg(cmd, s, spriv->num_sg, i) {
> + s->dma_address = (uintptr_t)sg_virt(s);
> + total += sg_dma_len(s);
> }
> } else {
> spriv->num_sg = scsi_dma_map(cmd);
> - for (i = 0; i < spriv->num_sg; i++)
> - total += sg_dma_len(&sg[i]);
> + scsi_for_each_sg(cmd, s, spriv->num_sg, i)
> + total += sg_dma_len(s);
> }
> spriv->cur_residue = sg_dma_len(sg);
> + spriv->prv_sg = NULL;
> spriv->cur_sg = sg;
> spriv->tot_residue = total;
> }
> @@ -444,7 +447,8 @@ static void esp_advance_dma(struct esp *esp, struct esp_cmd_entry *ent,
> p->tot_residue = 0;
> }
> if (!p->cur_residue && p->tot_residue) {
> - p->cur_sg++;
> + p->prv_sg = p->cur_sg;
> + p->cur_sg = sg_next(p->cur_sg);
> p->cur_residue = sg_dma_len(p->cur_sg);
> }
> }
> @@ -465,6 +469,7 @@ static void esp_save_pointers(struct esp *esp, struct esp_cmd_entry *ent)
> return;
> }
> ent->saved_cur_residue = spriv->cur_residue;
> + ent->saved_prv_sg = spriv->prv_sg;
> ent->saved_cur_sg = spriv->cur_sg;
> ent->saved_tot_residue = spriv->tot_residue;
> }
> @@ -479,6 +484,7 @@ static void esp_restore_pointers(struct esp *esp, struct esp_cmd_entry *ent)
> return;
> }
> spriv->cur_residue = ent->saved_cur_residue;
> + spriv->prv_sg = ent->saved_prv_sg;
> spriv->cur_sg = ent->saved_cur_sg;
> spriv->tot_residue = ent->saved_tot_residue;
> }
> @@ -1647,7 +1653,7 @@ static int esp_msgin_process(struct esp *esp)
> spriv = ESP_CMD_PRIV(ent->cmd);
>
> if (spriv->cur_residue == sg_dma_len(spriv->cur_sg)) {
> - spriv->cur_sg--;
> + spriv->cur_sg = spriv->prv_sg;
> spriv->cur_residue = 1;
> } else
> spriv->cur_residue++;
> diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
> index aa87a6b72dcc..91b32f2a1a1b 100644
> --- a/drivers/scsi/esp_scsi.h
> +++ b/drivers/scsi/esp_scsi.h
> @@ -251,6 +251,7 @@
> struct esp_cmd_priv {
> int num_sg;
> int cur_residue;
> + struct scatterlist *prv_sg;
> struct scatterlist *cur_sg;
> int tot_residue;
> };
> @@ -273,6 +274,7 @@ struct esp_cmd_entry {
> struct scsi_cmnd *cmd;
>
> unsigned int saved_cur_residue;
> + struct scatterlist *saved_prv_sg;
> struct scatterlist *saved_cur_sg;
> unsigned int saved_tot_residue;
>
>
