[PATCH V2] nvme: Invalidate dcache before submitting admin cmd

[PATCH V2] nvme: Invalidate dcache before submitting admin cmd

[Michael Trimarchi]

From: Jagan Teki <jagan@amarulasolutions.com>

This patch try to avoids eviction of dirty lines during DMA
transfer. The code right now execute the following step:

- allocate the buffer
- start a dma operation using the non-coherent dma buffer
- invalidate cache lines associated with the buffer
- read the buffer

This can lead to reading back not valid information, because the cache
controller could evict dirty cache lines belonging to the buffer *after*
the DMA operation has started to fill the DRAM.
In order to avoid this, a new invalidation is required *before* starting
the DMA operation. The patch just adds an invalidation before submitting
the DMA command.

Example below shows the nvme disk scan result without the following
patch

=> nvme scan
nvme_get_info_from_identify: nn = 544502629, vwc = 100,
sn = dev_0T, mn = `?\?, fr = t_part, mdts = 105

So, invalidating the cache before submitting the admin command,
fix the cpu read.

Cc: Andr? Przywara <andre.przywara@arm.com>
Reported-by: Suniel Mahesh <sunil@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Suniel Mahesh <sunil@amarulasolutions.com>
---
V1 -> V2:
	Get feedback from Andr? Przywara and
	Bin Meng. They help me not only to write a proper commit
	message but even make in some English form.
---
 drivers/nvme/nvme.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 0357aba7f1..fc64d93ab8 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -466,6 +466,9 @@ int nvme_identify(struct nvme_dev *dev, unsigned nsid,
 
 	c.identify.cns = cpu_to_le32(cns);
 
+	invalidate_dcache_range(dma_addr,
+				dma_addr + sizeof(struct nvme_id_ctrl));
+
 	ret = nvme_submit_admin_cmd(dev, &c, NULL);
 	if (!ret)
 		invalidate_dcache_range(dma_addr,
-- 
2.17.1

[PATCH V2] nvme: Invalidate dcache before submitting admin cmd

[Bin Meng]

On Thu, Jun 11, 2020 at 3:18 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> From: Jagan Teki <jagan@amarulasolutions.com>
>
> This patch try to avoids eviction of dirty lines during DMA
> transfer. The code right now execute the following step:
>
> - allocate the buffer
> - start a dma operation using the non-coherent dma buffer
> - invalidate cache lines associated with the buffer
> - read the buffer
>
> This can lead to reading back not valid information, because the cache
> controller could evict dirty cache lines belonging to the buffer *after*
> the DMA operation has started to fill the DRAM.
> In order to avoid this, a new invalidation is required *before* starting
> the DMA operation. The patch just adds an invalidation before submitting
> the DMA command.
>
> Example below shows the nvme disk scan result without the following
> patch
>
> => nvme scan
> nvme_get_info_from_identify: nn = 544502629, vwc = 100,
> sn = dev_0T, mn = `?\?, fr = t_part, mdts = 105
>
> So, invalidating the cache before submitting the admin command,
> fix the cpu read.
>
> Cc: Andr? Przywara <andre.przywara@arm.com>
> Reported-by: Suniel Mahesh <sunil@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com>
> ---
> V1 -> V2:
>         Get feedback from Andr? Przywara and
>         Bin Meng. They help me not only to write a proper commit
>         message but even make in some English form.
> ---
>  drivers/nvme/nvme.c | 3 +++
>  1 file changed, 3 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH V2] nvme: Invalidate dcache before submitting admin cmd

[Tom Rini]

On Wed, Jun 10, 2020 at 09:18:23PM +0200, Michael Trimarchi wrote:

> From: Jagan Teki <jagan@amarulasolutions.com>
> 
> This patch try to avoids eviction of dirty lines during DMA
> transfer. The code right now execute the following step:
> 
> - allocate the buffer
> - start a dma operation using the non-coherent dma buffer
> - invalidate cache lines associated with the buffer
> - read the buffer
> 
> This can lead to reading back not valid information, because the cache
> controller could evict dirty cache lines belonging to the buffer *after*
> the DMA operation has started to fill the DRAM.
> In order to avoid this, a new invalidation is required *before* starting
> the DMA operation. The patch just adds an invalidation before submitting
> the DMA command.
> 
> Example below shows the nvme disk scan result without the following
> patch
> 
> => nvme scan
> nvme_get_info_from_identify: nn = 544502629, vwc = 100,
> sn = dev_0T, mn = `?\?, fr = t_part, mdts = 105
> 
> So, invalidating the cache before submitting the admin command,
> fix the cpu read.
> 
> Cc: Andr? Przywara <andre.przywara@arm.com>
> Reported-by: Suniel Mahesh <sunil@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Applied (back on June 23rd) to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200625/a587175d/attachment.sig>