scsi fixes for non cache-coherent architectures (for 4.15 and stable)

scsi fixes for non cache-coherent architectures (for 4.15 and stable)

[Christoph Hellwig]

Hi Martin,

this series fixes scsi and libsas operations on platforms with cache
incoherent dma operations.  Patch 2 and 3 are originally from Huacai Chen,
but I've modified patch two so that it doesn't require his
dma_get_cache_alignment() rework, which I'd rather get into the next
merge window after the required fixups, and which isn't suitable for
a stable backport.  Patch 1 is new from me and makes sure
dma_get_cache_alignment is always available.  As on of the dma-mapping
maintainers I'd be happy with it going in through the scsi tree.

[PATCH 1/3] dma-mapping: always provide dma_get_cache_alignment

[Christoph Hellwig]

Provide the dummy version of dma_get_cache_alignment that always returns 1
even if CONFIG_HAS_DMA is not set, so that drivers and subsystems can
use it without ifdefs.

Cc: stable@vger.kernel.org
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/dma-mapping.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e8f8e8fb244d..81ed9b2d84dc 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -704,7 +704,6 @@ static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
 	return ret;
 }
 
-#ifdef CONFIG_HAS_DMA
 static inline int dma_get_cache_alignment(void)
 {
 #ifdef ARCH_DMA_MINALIGN
@@ -712,7 +711,6 @@ static inline int dma_get_cache_alignment(void)
 #endif
 	return 1;
 }
-#endif
 
 /* flags for the coherent memory api */
 #define DMA_MEMORY_EXCLUSIVE		0x01
-- 
2.14.2

[PATCH 2/3] scsi: use dma_get_cache_alignment() as minimum DMA alignment

[Christoph Hellwig]

From: Huacai Chen <chenhc-h23VmSynlr/QT0dZR+AlfA@public.gmane.org>

In non-coherent DMA mode, kernel uses cache flushing operations to maintain
I/O coherency, so scsi's block queue should be aligned to the value
returned by dma_get_cache_alignment().  Otherwise, If a DMA buffer and a
kernel structure share a same cache line, and if the kernel structure has
dirty data, cache_invalidate (no writeback) will cause data corruption.

Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Huacai Chen <chenhc-h23VmSynlr/QT0dZR+AlfA@public.gmane.org>
[hch: rebased and updated the comment and changelog]
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 drivers/scsi/scsi_lib.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1cbc497e00bd..00742c50cd44 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2148,11 +2148,13 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 		q->limits.cluster = 0;
 
 	/*
-	 * set a reasonable default alignment on word boundaries: the
-	 * host and device may alter it using
-	 * blk_queue_update_dma_alignment() later.
+	 * Set a reasonable default alignment:  The larger of 32-byte (dword),
+	 * which is a common minimum for HBAs, and the minimum DMA alignment,
+	 * which is set by the platform.
+	 *
+	 * Devices that require a bigger alignment can increase it later.
 	 */
-	blk_queue_dma_alignment(q, 0x03);
+	blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment()) - 1);
 }
 EXPORT_SYMBOL_GPL(__scsi_init_queue);
 
-- 
2.14.2

[PATCH 2/3] scsi: use dma_get_cache_alignment() as minimum DMA alignment

[Christoph Hellwig]

From: Huacai Chen <chenhc@lemote.com>

In non-coherent DMA mode, kernel uses cache flushing operations to maintain
I/O coherency, so scsi's block queue should be aligned to the value
returned by dma_get_cache_alignment().  Otherwise, If a DMA buffer and a
kernel structure share a same cache line, and if the kernel structure has
dirty data, cache_invalidate (no writeback) will cause data corruption.

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhc@lemote.com>
[hch: rebased and updated the comment and changelog]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1cbc497e00bd..00742c50cd44 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2148,11 +2148,13 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 		q->limits.cluster = 0;
 
 	/*
-	 * set a reasonable default alignment on word boundaries: the
-	 * host and device may alter it using
-	 * blk_queue_update_dma_alignment() later.
+	 * Set a reasonable default alignment:  The larger of 32-byte (dword),
+	 * which is a common minimum for HBAs, and the minimum DMA alignment,
+	 * which is set by the platform.
+	 *
+	 * Devices that require a bigger alignment can increase it later.
 	 */
-	blk_queue_dma_alignment(q, 0x03);
+	blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment()) - 1);
 }
 EXPORT_SYMBOL_GPL(__scsi_init_queue);
 
-- 
2.14.2

[PATCH 3/3] libsas: align sata_device's rps_resp on a cacheline

[Christoph Hellwig]

From: Huacai Chen <chenhc@lemote.com>

The rps_resp buffer in ata_device is a DMA target, but it isn't
explicitly cacheline aligned. Due to this, adjacent fields can be
overwritten with stale data from memory on non-coherent architectures.
As a result, the kernel is sometimes unable to communicate with an
SATA device behind a SAS expander.

Fix this by ensuring that the rps_resp buffer is cacheline aligned.

This issue is similar to that fixed by Commit 84bda12af31f93 ("libata:
align ap->sector_buf") and Commit 4ee34ea3a12396f35b26 ("libata: Align
ata_device's id on a cacheline").

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhc@lemote.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/scsi/libsas.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 0f9cbf96c093..6df6fe0c2198 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -159,11 +159,11 @@ struct expander_device {
 
 struct sata_device {
 	unsigned int class;
-	struct smp_resp        rps_resp; /* report_phy_sata_resp */
 	u8     port_no;        /* port number, if this is a PM (Port) */
 
 	struct ata_port *ap;
 	struct ata_host ata_host;
+	struct smp_resp rps_resp ____cacheline_aligned; /* report_phy_sata_resp */
 	u8     fis[ATA_RESP_FIS_SIZE];
 };
 
-- 
2.14.2

Re: scsi fixes for non cache-coherent architectures (for 4.15 and stable)

[Martin K. Petersen]

Christoph,

> this series fixes scsi and libsas operations on platforms with cache
> incoherent dma operations.  Patch 2 and 3 are originally from Huacai Chen,
> but I've modified patch two so that it doesn't require his
> dma_get_cache_alignment() rework, which I'd rather get into the next
> merge window after the required fixups, and which isn't suitable for
> a stable backport.  Patch 1 is new from me and makes sure
> dma_get_cache_alignment is always available.  As on of the dma-mapping
> maintainers I'd be happy with it going in through the scsi tree.

Applied to 4.15/scsi-fixes. Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering