[PATCH 0/5] mpt3sas: Fix changing coherent mask after allocation.

[PATCH 0/5] mpt3sas: Fix changing coherent mask after allocation.

[suganath-prabu.subramani]

From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

* Enables 64-bit dma for RDPQ and memory allocation 
within same 4gb region.
* With this there is no need to change DMA coherent
mask when there are outstanding allocations in mpt3sas.
* Code-Refactoring

Suganath Prabu S (5):
  mpt3sas: Don't change the dma coherent mask after      allocations
  mpt3sas: Rename function name is_MSB_are_same
  mpt3sas: Code Refactoring.
  mpt3sas: Handle RDPQ DMA allocation in same 4g region
  mpt3sas: Update version to 33.101.00.00

 drivers/scsi/mpt3sas/mpt3sas_base.c | 287 ++++++++++++++++++++++--------------
 drivers/scsi/mpt3sas/mpt3sas_base.h |   9 +-
 2 files changed, 181 insertions(+), 115 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] mpt3sas: Don't change the dma coherent mask after allocations

[suganath-prabu.subramani]

From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

During driver load ioc->dma_mask is set to 0. Since this
flag is not set, in _base_config_dma_addressing() driver
always sets 32 bit DMA and later after allocating memory
for RDPQ's the dma mask is set to 64/63 bit from
_base_change_consistent_dma_mask.

Removed Flag ioc->dma_mask and
_base_change_consistent_dma_mask().

Set coherent dma mask to 64/63/32 bit based on
controller at driver load time in _base_config_dma_addressing().
and If 63/64 bit fails attempt again with 32-bit DMA mask.

Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 82 ++++++++++++++-----------------------
 drivers/scsi/mpt3sas/mpt3sas_base.h |  2 -
 2 files changed, 30 insertions(+), 54 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 663782b..18c5045 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2806,55 +2806,42 @@ _base_build_sg_ieee(struct MPT3SAS_ADAPTER *ioc, void *psge,
 static int
 _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;
-
+	char *desc = "64";
+	u64 consistent_dma_mask = DMA_BIT_MASK(64);
 	if (ioc->is_mcpu_endpoint)
 		goto try_32bit;
 
-	required_mask = dma_get_required_mask(&pdev->dev);
-	if (sizeof(dma_addr_t) == 4 || required_mask == 32)
-		goto try_32bit;
-
-	if (ioc->dma_mask)
-		coherent_mask = DMA_BIT_MASK(dma_mask);
-	else
-		coherent_mask = DMA_BIT_MASK(32);
-
-	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 = dma_mask;
-	goto out;
-
- try_32bit:
-	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
+	/* Set 63 bit DMA mask for all SAS3 and SAS35 controllers */
+	if (ioc->hba_mpi_version_belonged > MPI2_VERSION) {
+		consistent_dma_mask = DMA_BIT_MASK(63);
+		desc = "63";
+	}
+	if (sizeof(dma_addr_t) > 4) {
+		const u64 required_mask = dma_get_required_mask(&pdev->dev);
+		if ((required_mask > DMA_BIT_MASK(32)) &&
+		    !pci_set_dma_mask(pdev, consistent_dma_mask) &&
+		    !pci_set_consistent_dma_mask(pdev,
+		    consistent_dma_mask)) {
+			ioc->base_add_sg_single = &_base_add_sg_single_64;
+			ioc->sge_size = sizeof(Mpi2SGESimple64_t);
+			goto out;
+		}
+	}
+try_32bit:
+	if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(32))
+	    && !pci_set_consistent_dma_mask(pdev,
+	    DMA_BIT_MASK(32))) {
+		ioc->base_add_sg_single = &_base_add_sg_single_32;
+		ioc->sge_size = sizeof(Mpi2SGESimple32_t);
+		desc = "32";
+	} else
 		return -ENODEV;
-
-	ioc->base_add_sg_single = &_base_add_sg_single_32;
-	ioc->sge_size = sizeof(Mpi2SGESimple32_t);
-	ioc->dma_mask = 32;
- out:
+out:
 	si_meminfo(&s);
-	ioc_info(ioc, "%d BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (%ld kB)\n",
-		 ioc->dma_mask, convert_to_kb(s.totalram));
-
-	return 0;
-}
-
-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(ioc->dma_mask))) {
-		if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32)))
-			return -ENODEV;
-	}
+	ioc_info(ioc,
+		"%s BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (%ld kB)\n",
+		desc, convert_to_kb(s.totalram));
 	return 0;
 }
 
@@ -5169,14 +5156,6 @@ _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 > 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));
-			goto out;
-		}
-	}
-
 	ioc->scsiio_depth = ioc->hba_queue_depth -
 	    ioc->hi_priority_depth - ioc->internal_depth;
 
@@ -7158,7 +7137,6 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
 	ioc->smp_affinity_enable = smp_affinity_enable;
 
 	ioc->rdpq_array_enable_assigned = 0;
-	ioc->dma_mask = 0;
 	if (ioc->is_aero_ioc)
 		ioc->base_readl = &_base_readl_aero;
 	else
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index e719715..caae040 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1026,7 +1026,6 @@ typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct MPT3SAS_ADAPTER *ioc);
  * @ir_firmware: IR firmware present
  * @bars: bitmask of BAR's that must be configured
  * @mask_interrupts: ignore interrupt
- * @dma_mask: used to set the consistent dma mask
  * @pci_access_mutex: Mutex to synchronize ioctl, sysfs show path and
  *			pci resource handling
  * @fault_reset_work_q_name: fw fault work queue
@@ -1205,7 +1204,6 @@ struct MPT3SAS_ADAPTER {
 	u8		ir_firmware;
 	int		bars;
 	u8		mask_interrupts;
-	int		dma_mask;
 
 	/* fw fault handler */
 	char		fault_reset_work_q_name[20];
-- 
1.8.3.1

Re: [PATCH 1/5] mpt3sas: Don't change the dma coherent mask after allocations

[Christoph Hellwig]

On Tue, Feb 11, 2020 at 05:18:09AM -0500, suganath-prabu.subramani@broadcom.com wrote:
> From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> 
> During driver load ioc->dma_mask is set to 0. Since this
> flag is not set, in _base_config_dma_addressing() driver
> always sets 32 bit DMA and later after allocating memory
> for RDPQ's the dma mask is set to 64/63 bit from
> _base_change_consistent_dma_mask.

Please use up 74 chars for your commit messages.

> +	u64 consistent_dma_mask = DMA_BIT_MASK(64);
>  	if (ioc->is_mcpu_endpoint)

Please keep an empty line after variable declarations.

> +	if (sizeof(dma_addr_t) > 4) {
> +		const u64 required_mask = dma_get_required_mask(&pdev->dev);
> +		if ((required_mask > DMA_BIT_MASK(32)) &&

No need for the braces.

> +		    !pci_set_dma_mask(pdev, consistent_dma_mask) &&
> +		    !pci_set_consistent_dma_mask(pdev,

Please do not use the pci_* functions in new code, but use the
generic DMA ones.

> +		    consistent_dma_mask)) {
> +			ioc->base_add_sg_single = &_base_add_sg_single_64;
> +			ioc->sge_size = sizeof(Mpi2SGESimple64_t);
> +			goto out;
> +		}
> +	}
> +try_32bit:
> +	if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(32))
> +	    && !pci_set_consistent_dma_mask(pdev,

Wrong coding style.  Also there is no need to fall back to a smaller
mask in modern kernels ever.

In other words: please drop this patch and use the one I submitted
weeks ago, as it gets all these things right.

[PATCH 2/5] mpt3sas: Rename function name is_MSB_are_same

[suganath-prabu.subramani]

From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

Renamed is_MSB_are_same() to mpt3sas_check_same_4gb_region()
for better readability.

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

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 18c5045..4718b86 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4922,7 +4922,7 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 }
 
 /**
- * is_MSB_are_same - checks whether all reply queues in a set are
+ * mpt3sas_check_same_4gb_region - checks whether all reply queues in a set are
  *	having same upper 32bits in their base memory address.
  * @reply_pool_start_address: Base address of a reply queue set
  * @pool_sz: Size of single Reply Descriptor Post Queues pool size
@@ -4932,7 +4932,7 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
  */
 
 static int
-is_MSB_are_same(long reply_pool_start_address, u32 pool_sz)
+mpt3sas_check_same_4gb_region(long reply_pool_start_address, u32 pool_sz)
 {
 	long reply_pool_end_address;
 
@@ -5384,7 +5384,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 	 * Actual requirement is not alignment, but we need start and end of
 	 * DMA address must have same upper 32 bit address.
 	 */
-	if (!is_MSB_are_same((long)ioc->sense, sz)) {
+	if (!mpt3sas_check_same_4gb_region((long)ioc->sense, sz)) {
 		//Release Sense pool & Reallocate
 		dma_pool_free(ioc->sense_dma_pool, ioc->sense, ioc->sense_dma);
 		dma_pool_destroy(ioc->sense_dma_pool);
-- 
1.8.3.1

[PATCH 3/5] mpt3sas: Code Refactoring.

[suganath-prabu.subramani]

From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

Separate out RDPQ allocations to
new function base_alloc_rdpq_dma_pool().

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

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 4718b86..8739310 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4946,6 +4946,55 @@ mpt3sas_check_same_4gb_region(long reply_pool_start_address, u32 pool_sz)
 }
 
 /**
+ * base_alloc_rdpq_dma_pool - Allocating DMA'able memory
+ *			for reply queues.
+ * @ioc: er object
+ * @sz: DMA Pool size
+ * Return: 0 for success, non-zero for failure.
+ */
+static int
+base_alloc_rdpq_dma_pool(struct MPT3SAS_ADAPTER *ioc, int sz)
+{
+	int i;
+
+	ioc->reply_post = kcalloc((ioc->rdpq_array_enable) ?
+	    (ioc->reply_queue_count):1,
+	    sizeof(struct reply_post_struct), GFP_KERNEL);
+
+	if (!ioc->reply_post) {
+		ioc_err(ioc, "reply_post_free pool: kcalloc failed\n");
+		return -ENOMEM;
+	}
+	ioc->reply_post_free_dma_pool = dma_pool_create("reply_post_free pool",
+	    &ioc->pdev->dev, sz, 16, 0);
+	if (!ioc->reply_post_free_dma_pool) {
+		ioc_err(ioc, "reply_post_free pool: dma_pool_create failed\n");
+		return -ENOMEM;
+	}
+	i = 0;
+	do {
+		ioc->reply_post[i].reply_post_free =
+		    dma_pool_zalloc(ioc->reply_post_free_dma_pool,
+		    GFP_KERNEL,
+		    &ioc->reply_post[i].reply_post_free_dma);
+		if (!ioc->reply_post[i].reply_post_free) {
+			ioc_err(ioc, "reply_post_free pool: dma_pool_alloc failed\n");
+			return -ENOMEM;
+		}
+		dinitprintk(ioc,
+		    ioc_info(ioc, "reply post free pool (0x%p): depth(%d), element_size(%d), pool_size(%d kB)\n",
+		    ioc->reply_post[i].reply_post_free,
+		    ioc->reply_post_queue_depth,
+		    8, sz / 1024));
+		dinitprintk(ioc,
+		    ioc_info(ioc, "reply_post_free_dma = (0x%llx)\n",
+		    (u64)ioc->reply_post[i].reply_post_free_dma));
+
+	} while (ioc->rdpq_array_enable && (++i < ioc->reply_queue_count));
+	return 0;
+}
+
+/**
  * _base_allocate_memory_pools - allocate start of day memory pools
  * @ioc: per adapter object
  *
@@ -5112,49 +5161,15 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 	    ioc->max_sges_in_chain_message,
 	    ioc->shost->sg_tablesize,
 	    ioc->chains_needed_per_io);
-
 	/* reply post queue, 16 byte align */
 	reply_post_free_sz = ioc->reply_post_queue_depth *
 	    sizeof(Mpi2DefaultReplyDescriptor_t);
-
 	sz = reply_post_free_sz;
 	if (_base_is_controller_msix_enabled(ioc) && !ioc->rdpq_array_enable)
 		sz *= ioc->reply_queue_count;
-
-	ioc->reply_post = kcalloc((ioc->rdpq_array_enable) ?
-	    (ioc->reply_queue_count):1,
-	    sizeof(struct reply_post_struct), GFP_KERNEL);
-
-	if (!ioc->reply_post) {
-		ioc_err(ioc, "reply_post_free pool: kcalloc failed\n");
-		goto out;
-	}
-	ioc->reply_post_free_dma_pool = dma_pool_create("reply_post_free pool",
-	    &ioc->pdev->dev, sz, 16, 0);
-	if (!ioc->reply_post_free_dma_pool) {
-		ioc_err(ioc, "reply_post_free pool: dma_pool_create failed\n");
+	if (base_alloc_rdpq_dma_pool(ioc, sz))
 		goto out;
-	}
-	i = 0;
-	do {
-		ioc->reply_post[i].reply_post_free =
-		    dma_pool_zalloc(ioc->reply_post_free_dma_pool,
-		    GFP_KERNEL,
-		    &ioc->reply_post[i].reply_post_free_dma);
-		if (!ioc->reply_post[i].reply_post_free) {
-			ioc_err(ioc, "reply_post_free pool: dma_pool_alloc failed\n");
-			goto out;
-		}
-		dinitprintk(ioc,
-			    ioc_info(ioc, "reply post free pool (0x%p): depth(%d), element_size(%d), pool_size(%d kB)\n",
-				     ioc->reply_post[i].reply_post_free,
-				     ioc->reply_post_queue_depth,
-				     8, sz / 1024));
-		dinitprintk(ioc,
-			    ioc_info(ioc, "reply_post_free_dma = (0x%llx)\n",
-				     (u64)ioc->reply_post[i].reply_post_free_dma));
-		total_sz += sz;
-	} while (ioc->rdpq_array_enable && (++i < ioc->reply_queue_count));
+	total_sz += sz;
 
 	ioc->scsiio_depth = ioc->hba_queue_depth -
 	    ioc->hi_priority_depth - ioc->internal_depth;
@@ -5167,7 +5182,6 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 		    ioc_info(ioc, "scsi host: can_queue depth (%d)\n",
 			     ioc->shost->can_queue));
 
-
 	/* contiguous pool for request and chains, 16 byte align, one extra "
 	 * "frame for smid=0
 	 */
-- 
1.8.3.1

[PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[suganath-prabu.subramani]

From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

For INVADER_SERIES each set of 8 reply queues (0 - 7, 8 - 15,..)and
VENTURA_SERIES each set of 16 reply queues (0 - 15, 16 - 31,..)should
be within 4 GB boundary.Driver uses limitation of VENTURA_SERIES
to manage INVADER_SERIES as well. So here driver is allocating the DMA
able memory for RDPQ's accordingly.

For RDPQ buffers, driver creates two separate pci pool.
"reply_post_free_dma_pool" and "reply_post_free_dma_pool_align"
First driver tries allocating memory from the pool
"reply_post_free_dma_pool", if the requested allocation are
within same 4gb region then proceeds for next allocations.
If not, allocates from reply_post_free_dma_pool_align which is
size aligned and if success, it will always meet same 4gb region
requirement

Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 167 ++++++++++++++++++++++++++----------
 drivers/scsi/mpt3sas/mpt3sas_base.h |   3 +
 2 files changed, 123 insertions(+), 47 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 8739310..7d10dd8 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4814,8 +4814,8 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 {
 	int i = 0;
 	int j = 0;
+	int dma_alloc_count = 0;
 	struct chain_tracker *ct;
-	struct reply_post_struct *rps;
 
 	dexitprintk(ioc, ioc_info(ioc, "%s\n", __func__));
 
@@ -4857,29 +4857,35 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 	}
 
 	if (ioc->reply_post) {
-		do {
-			rps = &ioc->reply_post[i];
-			if (rps->reply_post_free) {
-				dma_pool_free(
-				    ioc->reply_post_free_dma_pool,
-				    rps->reply_post_free,
-				    rps->reply_post_free_dma);
-				dexitprintk(ioc,
-					    ioc_info(ioc, "reply_post_free_pool(0x%p): free\n",
-						     rps->reply_post_free));
-				rps->reply_post_free = NULL;
+		dma_alloc_count = DIV_ROUND_UP(ioc->reply_queue_count,
+				RDPQ_MAX_INDEX_IN_ONE_CHUNK);
+		for (i = 0; i < ioc->reply_queue_count; i++) {
+			if (i % RDPQ_MAX_INDEX_IN_ONE_CHUNK == 0
+							&& dma_alloc_count) {
+				if (ioc->reply_post[i].reply_post_free) {
+					pci_pool_free(
+					    ioc->reply_post[i].dma_pool,
+					    ioc->reply_post[i].reply_post_free,
+					ioc->reply_post[i].reply_post_free_dma);
+					dexitprintk(ioc, ioc_info(ioc,
+					   "reply_post_free_pool(0x%p): free\n",
+					   ioc->reply_post[i].reply_post_free));
+					ioc->reply_post[i].reply_post_free =
+									NULL;
+				}
+				--dma_alloc_count;
 			}
-		} while (ioc->rdpq_array_enable &&
-			   (++i < ioc->reply_queue_count));
+		}
+		dma_pool_destroy(ioc->reply_post_free_dma_pool);
+		dma_pool_destroy(ioc->reply_post_free_dma_pool_align);
 		if (ioc->reply_post_free_array &&
 			ioc->rdpq_array_enable) {
 			dma_pool_free(ioc->reply_post_free_array_dma_pool,
-				ioc->reply_post_free_array,
-				ioc->reply_post_free_array_dma);
+			    ioc->reply_post_free_array,
+			    ioc->reply_post_free_array_dma);
 			ioc->reply_post_free_array = NULL;
 		}
 		dma_pool_destroy(ioc->reply_post_free_array_dma_pool);
-		dma_pool_destroy(ioc->reply_post_free_dma_pool);
 		kfree(ioc->reply_post);
 	}
 
@@ -4955,42 +4961,108 @@ mpt3sas_check_same_4gb_region(long reply_pool_start_address, u32 pool_sz)
 static int
 base_alloc_rdpq_dma_pool(struct MPT3SAS_ADAPTER *ioc, int sz)
 {
-	int i;
+	int i = 0;
+	u32 dma_alloc_count = 0;
+	int reply_post_free_sz = ioc->reply_post_queue_depth *
+		sizeof(Mpi2DefaultReplyDescriptor_t);
 
 	ioc->reply_post = kcalloc((ioc->rdpq_array_enable) ?
-	    (ioc->reply_queue_count):1,
-	    sizeof(struct reply_post_struct), GFP_KERNEL);
-
+				(ioc->reply_queue_count):1,
+				sizeof(struct reply_post_struct), GFP_KERNEL);
 	if (!ioc->reply_post) {
 		ioc_err(ioc, "reply_post_free pool: kcalloc failed\n");
 		return -ENOMEM;
 	}
-	ioc->reply_post_free_dma_pool = dma_pool_create("reply_post_free pool",
-	    &ioc->pdev->dev, sz, 16, 0);
-	if (!ioc->reply_post_free_dma_pool) {
+	/*
+	 *  For INVADER_SERIES each set of 8 reply queues(0-7, 8-15, ..) and
+	 *  VENTURA_SERIES each set of 16 reply queues(0-15, 16-31, ..) should
+	 *  be within 4GB boundary and also reply queues in a set must have same
+	 *  upper 32-bits in their memory address. so here driver is allocating
+	 *  the DMA'able memory for reply queues according.
+	 *  Driver uses limitation of
+	 *  VENTURA_SERIES to manage INVADER_SERIES as well.
+	 */
+	dma_alloc_count = DIV_ROUND_UP(ioc->reply_queue_count,
+				RDPQ_MAX_INDEX_IN_ONE_CHUNK);
+	ioc->reply_post_free_dma_pool =
+		dma_pool_create("reply_post_free pool",
+		    &ioc->pdev->dev, sz, 16, 0);
+	ioc->reply_post_free_dma_pool_align =
+		dma_pool_create("reply_post_free pool",
+		    &ioc->pdev->dev, sz, roundup_pow_of_two(sz), 0);
+	if (!ioc->reply_post_free_dma_pool ||
+	     !ioc->reply_post_free_dma_pool_align) {
 		ioc_err(ioc, "reply_post_free pool: dma_pool_create failed\n");
 		return -ENOMEM;
 	}
-	i = 0;
-	do {
-		ioc->reply_post[i].reply_post_free =
-		    dma_pool_zalloc(ioc->reply_post_free_dma_pool,
-		    GFP_KERNEL,
-		    &ioc->reply_post[i].reply_post_free_dma);
-		if (!ioc->reply_post[i].reply_post_free) {
-			ioc_err(ioc, "reply_post_free pool: dma_pool_alloc failed\n");
-			return -ENOMEM;
-		}
-		dinitprintk(ioc,
-		    ioc_info(ioc, "reply post free pool (0x%p): depth(%d), element_size(%d), pool_size(%d kB)\n",
-		    ioc->reply_post[i].reply_post_free,
-		    ioc->reply_post_queue_depth,
-		    8, sz / 1024));
-		dinitprintk(ioc,
-		    ioc_info(ioc, "reply_post_free_dma = (0x%llx)\n",
-		    (u64)ioc->reply_post[i].reply_post_free_dma));
+	for (i = 0; i < ioc->reply_queue_count; i++) {
+		if ((i % RDPQ_MAX_INDEX_IN_ONE_CHUNK == 0) && dma_alloc_count) {
+			ioc->reply_post[i].reply_post_free =
+				dma_pool_alloc(ioc->reply_post_free_dma_pool,
+				    GFP_KERNEL,
+				    &ioc->reply_post[i].reply_post_free_dma);
+			if (!ioc->reply_post[i].reply_post_free) {
+				ioc_err(ioc, "reply_post_free pool: "
+				    "dma_pool_alloc failed\n");
+				return -ENOMEM;
+			}
+		/* reply desc pool requires to be in same 4 gb region.
+		 * Below function will check this.
+		 * In case of failure, new pci pool will be created with updated
+		 * alignment.
+		 * For RDPQ buffers, driver allocates two separate pci pool.
+		 * Alignment will be used such a way that next allocation if
+		 * success, will always meet same 4gb region requirement.
+		 * Flag dma_pool keeps track of each buffers pool,
+		 * It will help driver while freeing the resources.
+		 */
+			if (!mpt3sas_check_same_4gb_region(
+				(long)ioc->reply_post[i].reply_post_free, sz)) {
+				dinitprintk(ioc,
+				    ioc_err(ioc, "bad Replypost free pool(0x%p)"
+				    "reply_post_free_dma = (0x%llx)\n",
+				    ioc->reply_post[i].reply_post_free,
+				    (unsigned long long)
+				    ioc->reply_post[i].reply_post_free_dma));
+
+				pci_pool_free(ioc->reply_post_free_dma_pool,
+				    ioc->reply_post[i].reply_post_free,
+				    ioc->reply_post[i].reply_post_free_dma);
+
+				ioc->reply_post[i].reply_post_free = NULL;
+				ioc->reply_post[i].reply_post_free_dma = 0;
+				//Retry with modified alignment
+				ioc->reply_post[i].reply_post_free =
+					dma_pool_alloc(
+					    ioc->reply_post_free_dma_pool_align,
+					    GFP_KERNEL,
+				    &ioc->reply_post[i].reply_post_free_dma);
+				if (!ioc->reply_post[i].reply_post_free) {
+					ioc_err(ioc, "reply_post_free pool: "
+					    "pci_pool_alloc failed\n");
+					return -ENOMEM;
+				}
+				ioc->reply_post[i].dma_pool =
+				    ioc->reply_post_free_dma_pool_align;
+			} else
+				ioc->reply_post[i].dma_pool =
+				    ioc->reply_post_free_dma_pool;
+			memset(ioc->reply_post[i].reply_post_free, 0,
+			    RDPQ_MAX_INDEX_IN_ONE_CHUNK *
+			    reply_post_free_sz);
+			dma_alloc_count--;
 
-	} while (ioc->rdpq_array_enable && (++i < ioc->reply_queue_count));
+		} else {
+			ioc->reply_post[i].reply_post_free =
+			    (Mpi2ReplyDescriptorsUnion_t *)
+			    ((long)ioc->reply_post[i-1].reply_post_free
+			    + reply_post_free_sz);
+			ioc->reply_post[i].reply_post_free_dma =
+			    (dma_addr_t)
+			    (ioc->reply_post[i-1].reply_post_free_dma +
+			    reply_post_free_sz);
+		}
+	}
 	return 0;
 }
 
@@ -5008,6 +5080,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 	u16 chains_needed_per_io;
 	u32 sz, total_sz, reply_post_free_sz, reply_post_free_array_sz;
 	u32 retry_sz;
+	u32 rdpq_sz = 0;
 	u16 max_request_credit, nvme_blocks_needed;
 	unsigned short sg_tablesize;
 	u16 sge_size;
@@ -5164,12 +5237,12 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 	/* reply post queue, 16 byte align */
 	reply_post_free_sz = ioc->reply_post_queue_depth *
 	    sizeof(Mpi2DefaultReplyDescriptor_t);
-	sz = reply_post_free_sz;
+	rdpq_sz = reply_post_free_sz * RDPQ_MAX_INDEX_IN_ONE_CHUNK;
 	if (_base_is_controller_msix_enabled(ioc) && !ioc->rdpq_array_enable)
-		sz *= ioc->reply_queue_count;
-	if (base_alloc_rdpq_dma_pool(ioc, sz))
+		rdpq_sz = reply_post_free_sz * ioc->reply_queue_count;
+	if (base_alloc_rdpq_dma_pool(ioc, rdpq_sz))
 		goto out;
-	total_sz += sz;
+	total_sz += rdpq_sz;
 
 	ioc->scsiio_depth = ioc->hba_queue_depth -
 	    ioc->hi_priority_depth - ioc->internal_depth;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index caae040..30ca583 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -367,6 +367,7 @@ struct mpt3sas_nvme_cmd {
 #define MPT3SAS_HIGH_IOPS_REPLY_QUEUES		8
 #define MPT3SAS_HIGH_IOPS_BATCH_COUNT		16
 #define MPT3SAS_GEN35_MAX_MSIX_QUEUES		128
+#define RDPQ_MAX_INDEX_IN_ONE_CHUNK		16
 
 /* OEM Specific Flags will come from OEM specific header files */
 struct Mpi2ManufacturingPage10_t {
@@ -1006,6 +1007,7 @@ struct mpt3sas_port_facts {
 struct reply_post_struct {
 	Mpi2ReplyDescriptorsUnion_t	*reply_post_free;
 	dma_addr_t			reply_post_free_dma;
+	struct dma_pool			*dma_pool;
 };
 
 typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct MPT3SAS_ADAPTER *ioc);
@@ -1423,6 +1425,7 @@ struct MPT3SAS_ADAPTER {
 	u8		rdpq_array_enable;
 	u8		rdpq_array_enable_assigned;
 	struct dma_pool *reply_post_free_dma_pool;
+	struct dma_pool	*reply_post_free_dma_pool_align;
 	struct dma_pool *reply_post_free_array_dma_pool;
 	Mpi2IOCInitRDPQArrayEntry *reply_post_free_array;
 	dma_addr_t reply_post_free_array_dma;
-- 
1.8.3.1

Re: [PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[Christoph Hellwig]

On Tue, Feb 11, 2020 at 05:18:12AM -0500, suganath-prabu.subramani@broadcom.com wrote:
> From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> 
> For INVADER_SERIES each set of 8 reply queues (0 - 7, 8 - 15,..)and
> VENTURA_SERIES each set of 16 reply queues (0 - 15, 16 - 31,..)should
> be within 4 GB boundary.Driver uses limitation of VENTURA_SERIES
> to manage INVADER_SERIES as well. So here driver is allocating the DMA
> able memory for RDPQ's accordingly.
> 
> For RDPQ buffers, driver creates two separate pci pool.
> "reply_post_free_dma_pool" and "reply_post_free_dma_pool_align"
> First driver tries allocating memory from the pool
> "reply_post_free_dma_pool", if the requested allocation are
> within same 4gb region then proceeds for next allocations.
> If not, allocates from reply_post_free_dma_pool_align which is
> size aligned and if success, it will always meet same 4gb region
> requirement

I don't fully understand the changelog here, and how having two
dma pools including one aligned is all that good.

Why not do a single dma_alloc_coherent and then subdvide it given
that all the allocations from the DMA pool seem to happen at HBA
initialization time anyway, invalidating the need for the dynamic
nature of the dma pools.

Re: [PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[Sreekanth Reddy]

On Wed, Feb 26, 2020 at 12:12 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Feb 11, 2020 at 05:18:12AM -0500, suganath-prabu.subramani@broadcom.com wrote:
> > From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> >
> > For INVADER_SERIES each set of 8 reply queues (0 - 7, 8 - 15,..)and
> > VENTURA_SERIES each set of 16 reply queues (0 - 15, 16 - 31,..)should
> > be within 4 GB boundary.Driver uses limitation of VENTURA_SERIES
> > to manage INVADER_SERIES as well. So here driver is allocating the DMA
> > able memory for RDPQ's accordingly.
> >
> > For RDPQ buffers, driver creates two separate pci pool.
> > "reply_post_free_dma_pool" and "reply_post_free_dma_pool_align"
> > First driver tries allocating memory from the pool
> > "reply_post_free_dma_pool", if the requested allocation are
> > within same 4gb region then proceeds for next allocations.
> > If not, allocates from reply_post_free_dma_pool_align which is
> > size aligned and if success, it will always meet same 4gb region
> > requirement
>
> I don't fully understand the changelog here, and how having two
> dma pools including one aligned is all that good.

The requirement is that driver needs a set of memory blocks of size
~106 KB and this block should not cross the 4gb boundary (i.e.
starting & end address of this block should have the same higher 32
bit address). So what we are doing is that first we allocate a block
from generic pool 'reply_post_free_dma_pool' and we check whether this
block cross the 4gb boundary or not, if it is yes then we free this
block and we try to allocate block once gain from pool
'reply_post_free_dma_pool_align' where we alignment of this pool is
set to power of two from block size. Hoping that second time
allocation block will not cross the 4 gb boundary.

Is there any interface or API which make sures that it always
allocates the required size memory block and also satisfies 4bg
boundary condtion?

>
> Why not do a single dma_alloc_coherent and then subdvide it given
> that all the allocations from the DMA pool seem to happen at HBA
> initialization time anyway, invalidating the need for the dynamic
> nature of the dma pools.

we need 8 blocks of block size ~106 KB, so total will be ~848 KB and
most of the times we may not get this much size continuous single
block memory and also this block should satisfy the 4gb boundary
requirement. And hence driver is allocating each block individually.

Regards,
Sreekanth

Re: [PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[Sreekanth Reddy]

Hi,

Any update over my previous reply?

Thanks,
Sreekanth

On Thu, Feb 27, 2020 at 6:11 PM Sreekanth Reddy
<sreekanth.reddy@broadcom.com> wrote:
>
> On Wed, Feb 26, 2020 at 12:12 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Tue, Feb 11, 2020 at 05:18:12AM -0500, suganath-prabu.subramani@broadcom.com wrote:
> > > From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> > >
> > > For INVADER_SERIES each set of 8 reply queues (0 - 7, 8 - 15,..)and
> > > VENTURA_SERIES each set of 16 reply queues (0 - 15, 16 - 31,..)should
> > > be within 4 GB boundary.Driver uses limitation of VENTURA_SERIES
> > > to manage INVADER_SERIES as well. So here driver is allocating the DMA
> > > able memory for RDPQ's accordingly.
> > >
> > > For RDPQ buffers, driver creates two separate pci pool.
> > > "reply_post_free_dma_pool" and "reply_post_free_dma_pool_align"
> > > First driver tries allocating memory from the pool
> > > "reply_post_free_dma_pool", if the requested allocation are
> > > within same 4gb region then proceeds for next allocations.
> > > If not, allocates from reply_post_free_dma_pool_align which is
> > > size aligned and if success, it will always meet same 4gb region
> > > requirement
> >
> > I don't fully understand the changelog here, and how having two
> > dma pools including one aligned is all that good.
>
> The requirement is that driver needs a set of memory blocks of size
> ~106 KB and this block should not cross the 4gb boundary (i.e.
> starting & end address of this block should have the same higher 32
> bit address). So what we are doing is that first we allocate a block
> from generic pool 'reply_post_free_dma_pool' and we check whether this
> block cross the 4gb boundary or not, if it is yes then we free this
> block and we try to allocate block once gain from pool
> 'reply_post_free_dma_pool_align' where we alignment of this pool is
> set to power of two from block size. Hoping that second time
> allocation block will not cross the 4 gb boundary.
>
> Is there any interface or API which make sures that it always
> allocates the required size memory block and also satisfies 4bg
> boundary condtion?
>
> >
> > Why not do a single dma_alloc_coherent and then subdvide it given
> > that all the allocations from the DMA pool seem to happen at HBA
> > initialization time anyway, invalidating the need for the dynamic
> > nature of the dma pools.
>
> we need 8 blocks of block size ~106 KB, so total will be ~848 KB and
> most of the times we may not get this much size continuous single
> block memory and also this block should satisfy the 4gb boundary
> requirement. And hence driver is allocating each block individually.
>
> Regards,
> Sreekanth

Re: [PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[Suganath Prabu Subramani]

Hi Christoph,

We will simplify the logic as below, let us know your comments.

#use one dma pool for RDPQ's, thus removes the logic of using second
dma pool with align.
The requirement is, RDPQ memory blocks starting & end address should
have the same
higher 32 bit address.

1) At driver load, set DMA Mask to 64 and allocate memory for RDPQ's.

2) Check if allocated resources are in the same 4GB range.

3) If #2 is true, continue with 64 bit DMA and go to #6

4) If #2 is false, then free all the resources from #1.

5) Set DMA mask to 32 and allocate RDPQ's.

6) Proceed with driver loading and other allocations.

Thanks,
Suganath


On Wed, Mar 18, 2020 at 12:21 PM Suganath Prabu Subramani
<suganath-prabu.subramani@broadcom.com> wrote:
>
> Hi Christoph,
>
> We will simplify the logic as below, let us know your comments.
>
> #use one dma pool for RDPQ's, thus removes the logic of using second dma pool with align.
> The requirement is, RDPQ memory blocks starting & end address should have the same
> higher 32 bit address.
>
> 1) At driver load, set DMA Mask to 64 and allocate memory for RDPQ's.
>
> 2) Check if allocated resources are in the same 4GB range.
>
> 3) If #2 is true, continue with 64 bit DMA and go to #6
>
> 4) If #2 is false, then free all the resources from #1.
>
> 5) Set DMA mask to 32 and allocate RDPQ's.
>
> 6) Proceed with driver loading and other allocations.
>
> Thanks,
> Suganath
>
> On Thu, Mar 5, 2020 at 2:40 PM Sreekanth Reddy <sreekanth.reddy@broadcom.com> wrote:
>>
>> Hi,
>>
>> Any update over my previous reply?
>>
>> Thanks,
>> Sreekanth
>>
>> On Thu, Feb 27, 2020 at 6:11 PM Sreekanth Reddy
>> <sreekanth.reddy@broadcom.com> wrote:
>> >
>> > On Wed, Feb 26, 2020 at 12:12 AM Christoph Hellwig <hch@infradead.org> wrote:
>> > >
>> > > On Tue, Feb 11, 2020 at 05:18:12AM -0500, suganath-prabu.subramani@broadcom.com wrote:
>> > > > From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
>> > > >
>> > > > For INVADER_SERIES each set of 8 reply queues (0 - 7, 8 - 15,..)and
>> > > > VENTURA_SERIES each set of 16 reply queues (0 - 15, 16 - 31,..)should
>> > > > be within 4 GB boundary.Driver uses limitation of VENTURA_SERIES
>> > > > to manage INVADER_SERIES as well. So here driver is allocating the DMA
>> > > > able memory for RDPQ's accordingly.
>> > > >
>> > > > For RDPQ buffers, driver creates two separate pci pool.
>> > > > "reply_post_free_dma_pool" and "reply_post_free_dma_pool_align"
>> > > > First driver tries allocating memory from the pool
>> > > > "reply_post_free_dma_pool", if the requested allocation are
>> > > > within same 4gb region then proceeds for next allocations.
>> > > > If not, allocates from reply_post_free_dma_pool_align which is
>> > > > size aligned and if success, it will always meet same 4gb region
>> > > > requirement
>> > >
>> > > I don't fully understand the changelog here, and how having two
>> > > dma pools including one aligned is all that good.
>> >
>> > The requirement is that driver needs a set of memory blocks of size
>> > ~106 KB and this block should not cross the 4gb boundary (i.e.
>> > starting & end address of this block should have the same higher 32
>> > bit address). So what we are doing is that first we allocate a block
>> > from generic pool 'reply_post_free_dma_pool' and we check whether this
>> > block cross the 4gb boundary or not, if it is yes then we free this
>> > block and we try to allocate block once gain from pool
>> > 'reply_post_free_dma_pool_align' where we alignment of this pool is
>> > set to power of two from block size. Hoping that second time
>> > allocation block will not cross the 4 gb boundary.
>> >
>> > Is there any interface or API which make sures that it always
>> > allocates the required size memory block and also satisfies 4bg
>> > boundary condtion?
>> >
>> > >
>> > > Why not do a single dma_alloc_coherent and then subdvide it given
>> > > that all the allocations from the DMA pool seem to happen at HBA
>> > > initialization time anyway, invalidating the need for the dynamic
>> > > nature of the dma pools.
>> >
>> > we need 8 blocks of block size ~106 KB, so total will be ~848 KB and
>> > most of the times we may not get this much size continuous single
>> > block memory and also this block should satisfy the 4gb boundary
>> > requirement. And hence driver is allocating each block individually.
>> >
>> > Regards,
>> > Sreekanth

Re: [PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[Christoph Hellwig]

On Wed, Mar 18, 2020 at 12:21:11PM +0530, Suganath Prabu Subramani wrote:
> Hi Christoph,
> 
> We will simplify the logic as below, let us know your comments.
> 
> #use one dma pool for RDPQ's, thus removes the logic of using second dma
> pool with align.
> The requirement is, RDPQ memory blocks starting & end address should have
> the same
> higher 32 bit address.
> 
> 1) At driver load, set DMA Mask to 64 and allocate memory for RDPQ's.
> 
> 2) Check if allocated resources are in the same 4GB range.
> 
> 3) If #2 is true, continue with 64 bit DMA and go to #6
> 
> 4) If #2 is false, then free all the resources from #1.
> 
> 5) Set DMA mask to 32 and allocate RDPQ's.
> 
> 6) Proceed with driver loading and other allocations.

Yes, please do.

Re: [PATCH 4/5] mpt3sas: Handle RDPQ DMA allocation in same 4g region

[Suganath Prabu Subramani]

Thank you,
We will post the patches in couple of days.


On Thu, Mar 19, 2020 at 6:39 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Mar 18, 2020 at 12:21:11PM +0530, Suganath Prabu Subramani wrote:
> > Hi Christoph,
> >
> > We will simplify the logic as below, let us know your comments.
> >
> > #use one dma pool for RDPQ's, thus removes the logic of using second dma
> > pool with align.
> > The requirement is, RDPQ memory blocks starting & end address should have
> > the same
> > higher 32 bit address.
> >
> > 1) At driver load, set DMA Mask to 64 and allocate memory for RDPQ's.
> >
> > 2) Check if allocated resources are in the same 4GB range.
> >
> > 3) If #2 is true, continue with 64 bit DMA and go to #6
> >
> > 4) If #2 is false, then free all the resources from #1.
> >
> > 5) Set DMA mask to 32 and allocate RDPQ's.
> >
> > 6) Proceed with driver loading and other allocations.
>
> Yes, please do.

[PATCH 5/5] mpt3sas: Update version to 33.101.00.00

[suganath-prabu.subramani]

From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

Update driver version from 33.100.00.00 to
33.101.00.00

Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 30ca583..25f1701 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -76,9 +76,9 @@
 #define MPT3SAS_DRIVER_NAME		"mpt3sas"
 #define MPT3SAS_AUTHOR "Avago Technologies <MPT-FusionLinux.pdl@avagotech.com>"
 #define MPT3SAS_DESCRIPTION	"LSI MPT Fusion SAS 3.0 Device Driver"
-#define MPT3SAS_DRIVER_VERSION		"33.100.00.00"
+#define MPT3SAS_DRIVER_VERSION		"33.101.00.00"
 #define MPT3SAS_MAJOR_VERSION		33
-#define MPT3SAS_MINOR_VERSION		100
+#define MPT3SAS_MINOR_VERSION		101
 #define MPT3SAS_BUILD_VERSION		0
 #define MPT3SAS_RELEASE_VERSION	00
 
-- 
1.8.3.1