On Fri, Sep 09, 2022 at 01:42:34PM -0600, Keith Busch wrote:

On Fri, Sep 09, 2022 at 10:19:15PM +0300, Serge Semin wrote:

quoted

Recent commit 52fde2c07da6 ("nvme: set dma alignment to dword") has caused

Folks, I've thoroughly studied the whole NVME subsystem looking for
similar problems. Turned out there is one more place which may cause the
same issue. It's connected with the opal_dev.{cmd,req} buffers passed to
the nvme_sec_submit() method. The rest of the buffers involved in the NVME
DMA are either allocated by kmalloc (must be cache-line-aligned by design)
or bounced-buffered if allocated on the stack (see the blk_rq_map_kern()
method implementation).

What about user space addresses?

Reasonable question. Alas I haven't researched the user-space part as
much thorough. What I can say for sure that we haven't detected any
unaligned buffers passed to the DMA-mapping procedure other than the
ones denoted in this patch and in the next one. So to speak so far
none of the NVME-involved user-space buffers have had unaligned offset
in the physical address space. I have merged in the next patch in our
local kernel tree:
https://patchwork.linux-mips.org/project/linux-mips/patch/20161125184611.28396-3-paul.burton@imgtec.com/
So if an unaligned buffer was passed we would have immediately got it
detected.

We can map those with cacheline offsets.

If we could do that easily it would have been great. But I don't see
an easy way out. AFAICS we'll need to fix the blk_rq_map_user_iov()
method so one would CPU-based copy the unaligned part of the buffer
and perform the DMA-required operations with the rest of it. Do you
have any better suggestion in mind?

quoted

---
 drivers/nvme/host/hwmon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
index 0a586d712920..94192ab7a02d 100644
--- a/drivers/nvme/host/hwmon.c
+++ b/drivers/nvme/host/hwmon.c

@@ -10,9 +10,10 @@
 
 #include "nvme.h"
 
+/* DMA-noncoherent platforms require the cache-aligned buffers */
 struct nvme_hwmon_data {
+	struct nvme_smart_log log ____cacheline_aligned;
 	struct nvme_ctrl *ctrl;
-	struct nvme_smart_log log;

So this by chance happened to work before 52fde2c07da6 because the field
started at a 4-byte offset on your arch?

Correct. The offset is 4-bytes indeed so the log-field base address is
4-bytes aligned. Due to that the bounce-buffer used to be used for the
NVME SMART log getting. Since the denoted commit the log-buffer have
been directly used for DMA, which has revealed the problem caused by the
cache-invalidation on the buffer mapping.

The change looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Thanks.

-Sergey