On Fri, Jan 17, 2020 at 7:15 PM Christoph Hellwig <hch@xxxxxx> wrote:
>
> The DMA layer does not allow changing the DMA coherent mask after
> there are outstanding allocations. Stop doing that and always
> use a 32-bit coherent DMA mask in mpt3sas.
Our HBA hardware has a requirement that each set of RDPQ reply
descriptor pools should be within the 4gb region. so to accommodate
this requirement driver is first setting the DMA coherent mask to 32
bit then allocating the RDPQ pools and then resetting the DMA coherent
make to 64 and allocating the remaining pools.
if we completely set the DMA coherent mask to 32 bit then there are
chances that sometimes driver may not get requested memory within the
first 4gb region and HBA initialization may fail. So instead of
setting the DMA coherent mask to 32 bit, we follow the same below
logic which megaraid_sas driver is doing now. i.e we first allocate
set of rdpq pools at once and check this allocated block is within the
4gb region, if not then free this block and allocate a new block from
aligned pci pool.
/* 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 always allocate two
separate pci pool.
* Alignment will be used such a way that next allocation if
* success, will always meet same 4gb region requirement.
* rdpq_tracker keep track of each buffer's physical,
* virtual address and pci pool descriptor. It will help driver
* while freeing the resources.
*
*/
if (!megasas_check_same_4gb_region(instance, rdpq_chunk_phys[i],
chunk_size)) {
dma_pool_free(fusion->reply_frames_desc_pool,
rdpq_chunk_virt[i],
rdpq_chunk_phys[i]);
rdpq_chunk_virt[i] =
dma_pool_alloc(fusion->reply_frames_desc_pool_align,
GFP_KERNEL, &rdpq_chunk_phys[i]);
if (!rdpq_chunk_virt[i]) {
dev_err(&instance->pdev->dev,
"Failed from %s %d\n",
__func__, __LINE__);
return -ENOMEM;
}
fusion->rdpq_tracker[i].dma_pool_ptr =
fusion->reply_frames_desc_pool_align;
} else {
fusion->rdpq_tracker[i].dma_pool_ptr =
fusion->reply_frames_desc_pool;
}
Next week I will post the patch with above logic implemented in mpt3sas driver.
Regards,
Sreekanth
>
> Reported-by: Abdul Haleem <abdhalee@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Tested-by: Abdul Haleem <abdhalee@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/scsi/mpt3sas/mpt3sas_base.c | 67 ++++++++---------------------
> drivers/scsi/mpt3sas/mpt3sas_base.h | 2 -
> 2 files changed, 19 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index fea3cb6a090b..3b51bed05008 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2706,58 +2706,38 @@ _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;
> -
> - if (ioc->is_mcpu_endpoint)
> - goto try_32bit;
> + int dma_mask;
>
> - 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);
> + if (ioc->is_mcpu_endpoint ||
> + sizeof(dma_addr_t) == 4 ||
> + dma_get_required_mask(&pdev->dev) <= 32)
> + dma_mask = 32;
> + /* Set 63 bit DMA mask for all SAS3 and SAS35 controllers */
> + else if (ioc->hba_mpi_version_belonged > MPI2_VERSION)
> + dma_mask = 63;
> else
> - coherent_mask = DMA_BIT_MASK(32);
> + dma_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 = dma_mask;
> - goto out;
> -
> - try_32bit:
> - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
> + dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)))
> return -ENODEV;
>
> - ioc->base_add_sg_single = &_base_add_sg_single_32;
> - ioc->sge_size = sizeof(Mpi2SGESimple32_t);
> - ioc->dma_mask = 32;
> - out:
> + if (dma_mask > 32) {
> + ioc->base_add_sg_single = &_base_add_sg_single_64;
> + ioc->sge_size = sizeof(Mpi2SGESimple64_t);
> + } else {
> + ioc->base_add_sg_single = &_base_add_sg_single_32;
> + ioc->sge_size = sizeof(Mpi2SGESimple32_t);
> + }
> +
> 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));
> + 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;
> - }
> - return 0;
> -}
> -
> /**
> * _base_check_enable_msix - checks MSIX capabable.
> * @ioc: per adapter object
> @@ -5030,14 +5010,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;
>
> @@ -6965,7 +6937,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 faca0a5e71f8..e57cade1155c 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -1011,7 +1011,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
> @@ -1185,7 +1184,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];
> --
> 2.24.1
>
