On Tue, 28 Sep 2021 10:01:40 +0200
Stefan Agner <[email protected]> wrote:
> The current code invalidates the range after the read buffer since the
> buffer pointer gets incremented in the read loop. Use a temporary
> pointer to make sure we have a pristine pointer to invalidate the
> correct memory range after read.
Ah, good catch!
This looks good, just one nit below:
> Fixes: 704e040a51d2 ("nvme: Apply cache operations on the DMA buffers")
> Signed-off-by: Stefan Agner <[email protected]>
> ---
>
> drivers/nvme/nvme.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index f6465ea7f4..354513ad30 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -743,6 +743,7 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t
> blknr,
> u64 prp2;
> u64 total_len = blkcnt << desc->log2blksz;
> u64 temp_len = total_len;
> + void *temp_buffer = buffer;
You could make this an unsigned long (or better uintptr_t), then lose all
the casts below and avoid the void ptr arithmetic.
But it works either way, so:
Reviewed-by: Andre Przywara <[email protected]>
Cheers,
Andre
>
> u64 slba = blknr;
> u16 lbas = 1 << (dev->max_transfer_shift - ns->lba_shift);
> @@ -770,19 +771,19 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t
> blknr,
> }
>
> if (nvme_setup_prps(dev, &prp2,
> - lbas << ns->lba_shift, (ulong)buffer))
> + lbas << ns->lba_shift, (ulong)temp_buffer))
> return -EIO;
> c.rw.slba = cpu_to_le64(slba);
> slba += lbas;
> c.rw.length = cpu_to_le16(lbas - 1);
> - c.rw.prp1 = cpu_to_le64((ulong)buffer);
> + c.rw.prp1 = cpu_to_le64((ulong)temp_buffer);
> c.rw.prp2 = cpu_to_le64(prp2);
> status = nvme_submit_sync_cmd(dev->queues[NVME_IO_Q],
> &c, NULL, IO_TIMEOUT);
> if (status)
> break;
> temp_len -= (u32)lbas << ns->lba_shift;
> - buffer += lbas << ns->lba_shift;
> + temp_buffer += lbas << ns->lba_shift;
> }
>
> if (read)
