diff mbox series

mpt3sas: Use 63-bit DMA addressing on SAS35 HBA

Message ID 1564135257-33188-1-git-send-email-suganath-prabu.subramani@broadcom.com (mailing list archive)
State Superseded
Headers show
Series mpt3sas: Use 63-bit DMA addressing on SAS35 HBA | expand

Commit Message

Suganath Prabu S July 26, 2019, 10 a.m. UTC
Although SAS3 & SAS3.5 IT HBA controllers support
64-bit DMA addressing, as per hardware design,
DMA address with all 64-bits set (0xFFFFFFFF-FFFFFFFF)
results in a firmware fault.

Fix:
Driver will set 63-bit DMA mask to ensure the above address
will not be used.

Signed-off-by: Suganath Prabu <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Greg Kroah-Hartman July 26, 2019, 10:20 a.m. UTC | #1
On Fri, Jul 26, 2019 at 06:00:57AM -0400, Suganath Prabu wrote:
> Although SAS3 & SAS3.5 IT HBA controllers support
> 64-bit DMA addressing, as per hardware design,
> DMA address with all 64-bits set (0xFFFFFFFF-FFFFFFFF)
> results in a firmware fault.
> 
> Fix:
> Driver will set 63-bit DMA mask to ensure the above address
> will not be used.
> 
> Signed-off-by: Suganath Prabu <suganath-prabu.subramani@broadcom.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>
Christoph Hellwig July 26, 2019, 2:27 p.m. UTC | #2
On Fri, Jul 26, 2019 at 06:00:57AM -0400, Suganath Prabu wrote:
> Although SAS3 & SAS3.5 IT HBA controllers support
> 64-bit DMA addressing, as per hardware design,
> DMA address with all 64-bits set (0xFFFFFFFF-FFFFFFFF)
> results in a firmware fault.

Linux will never send a dma address with all bits set anyway, as that
is our magic escape for the dma_addr_t error value.  Additionally to
generate that address you'd need a 1-byte sized, 1-byte aligned buffer,
which we never use.
Sreekanth Reddy July 29, 2019, 11:55 a.m. UTC | #3
On Fri, Jul 26, 2019 at 7:57 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Jul 26, 2019 at 06:00:57AM -0400, Suganath Prabu wrote:
> > Although SAS3 & SAS3.5 IT HBA controllers support
> > 64-bit DMA addressing, as per hardware design,
> > DMA address with all 64-bits set (0xFFFFFFFF-FFFFFFFF)
> > results in a firmware fault.
>
> Linux will never send a dma address with all bits set anyway, as that
> is our magic escape for the dma_addr_t error value.  Additionally to
> generate that address you'd need a 1-byte sized, 1-byte aligned buffer,
> which we never use.

I agree with your above statement. But it is also possible that
0xFFFFFFFF-FFFFFFFF falls under the DMA able range, e.g. SGE's start
address is 0xFFFFFFFF-FFFF000 and data length is 0x1000 bytes. So when
HBA tries to DMA the data at 0xFFFFFFFF-FFFFFFFF location then it will
faults the firmware due to it's hardware design.

We have observed above example's SGE address and length on AMD systems
with SME & IOMMU enabled.

Thanks,
Sreekanth
Christoph Hellwig July 30, 2019, 6:38 a.m. UTC | #4
On Mon, Jul 29, 2019 at 05:25:35PM +0530, Sreekanth Reddy wrote:
> 
> I agree with your above statement. But it is also possible that
> 0xFFFFFFFF-FFFFFFFF falls under the DMA able range, e.g. SGE's start
> address is 0xFFFFFFFF-FFFF000 and data length is 0x1000 bytes. So when
> HBA tries to DMA the data at 0xFFFFFFFF-FFFFFFFF location then it will
> faults the firmware due to it's hardware design.
> 
> We have observed above example's SGE address and length on AMD systems
> with SME & IOMMU enabled.

Ok.  Please slightly update the changelog to say dma ranges instead
of a dma address, as that implicies the addr field in the sg to me.

Otherwise looks ok:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 6846628..050c0f0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2703,6 +2703,8 @@  _base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc, struct pci_dev *pdev)
 {
 	u64 required_mask, coherent_mask;
 	struct sysinfo s;
+	/* Set 63 bit DMA mask for all SAS3 and SAS35 controllers */
+	int dma_mask = (ioc->hba_mpi_version_belonged > MPI2_VERSION) ? 63 : 64;
 
 	if (ioc->is_mcpu_endpoint)
 		goto try_32bit;
@@ -2712,17 +2714,17 @@  _base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc, struct pci_dev *pdev)
 		goto try_32bit;
 
 	if (ioc->dma_mask)
-		coherent_mask = DMA_BIT_MASK(64);
+		coherent_mask = DMA_BIT_MASK(dma_mask);
 	else
 		coherent_mask = DMA_BIT_MASK(32);
 
-	if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) ||
+	if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(dma_mask)) ||
 	    dma_set_coherent_mask(&pdev->dev, coherent_mask))
 		goto try_32bit;
 
 	ioc->base_add_sg_single = &_base_add_sg_single_64;
 	ioc->sge_size = sizeof(Mpi2SGESimple64_t);
-	ioc->dma_mask = 64;
+	ioc->dma_mask = dma_mask;
 	goto out;
 
  try_32bit:
@@ -2744,7 +2746,7 @@  static int
 _base_change_consistent_dma_mask(struct MPT3SAS_ADAPTER *ioc,
 				      struct pci_dev *pdev)
 {
-	if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {
+	if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(ioc->dma_mask))) {
 		if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32)))
 			return -ENODEV;
 	}
@@ -4989,7 +4991,7 @@  _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 		total_sz += sz;
 	} while (ioc->rdpq_array_enable && (++i < ioc->reply_queue_count));
 
-	if (ioc->dma_mask == 64) {
+	if (ioc->dma_mask > 32) {
 		if (_base_change_consistent_dma_mask(ioc, ioc->pdev) != 0) {
 			ioc_warn(ioc, "no suitable consistent DMA mask for %s\n",
 				 pci_name(ioc->pdev));