[Devel] [PATCH rh7 v2 3/3] scsi: mpt3sas: Don't widen coherent DMA mask after RDPQ allocation

[Konstantin Khorenko]

The mpt3sas firmware uses a "shared upper-32-bit base + 32-bit offset"
addressing model for several buffer types.  In the IOCInit message:

  - SenseBufferAddressHigh is a single 32-bit value for ALL sense buffers
  - SystemReplyAddressHigh is a single 32-bit value for ALL reply buffers

This means every coherent buffer pool (sense, reply, reply_free, chains,
PCIe SGL, RDPQ) must reside entirely within a single 4 GB region -
otherwise the firmware computes wrong addresses and corrupts memory,
causes double bio completions, etc.

The problem:

  _base_config_dma_addressing() correctly sets the coherent DMA mask to
  32-bit on the first call (when ioc->dma_mask == 0).  RDPQ pools are
  then allocated within that 32-bit space - fine.

  But immediately after RDPQ allocation, _base_change_consistent_dma_mask()
  widens the coherent mask back to 63/64-bit:

    if (ioc->dma_mask > 32)
        _base_change_consistent_dma_mask(ioc, ioc->pdev);
            -> pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(63))

  All subsequent coherent allocations - request pool, PCIe SGL pool,
  chain buffers, sense buffers, reply pool, reply_free pool - now use
  63/64-bit coherent DMA and can land above 4 GB or cross a 4 GB
  boundary.

  The existing is_MSB_are_same() check for the sense pool is insufficient:
  it only covers sense buffers (not reply, chain, etc.) and checks the
  virtual address instead of the DMA address (a separate bug).

The proper upstream fix is a series of 15 patches that add per-pool
boundary checks with allocate-check-retry logic:

  - scsi: mpt3sas: Rename function name is_MSB_are_same
  - scsi: mpt3sas: Don't change the DMA coherent mask after allocations
  - scsi: mpt3sas: Separate out RDPQ allocation to new function
  - scsi: mpt3sas: Handle RDPQ DMA allocation in same 4G region
  - scsi: mpt3sas: Fix ReplyPostFree pool allocation
  - scsi: mpt3sas: Force PCIe scatterlist allocations to be within same 4 GB region
  - scsi: mpt3sas: Force chain buffer allocations to be within same 4 GB region
  - scsi: mpt3sas: Force sense buffer allocations to be within same 4 GB region
  - scsi: mpt3sas: Force reply buffer allocations to be within same 4 GB region
  - scsi: mpt3sas: Force reply post buffer allocations to be within same 4 GB region
  - scsi: mpt3sas: Force reply post array allocations to be within same 4 GB region
  - scsi: mpt3sas: Fix incorrect 4GB boundary check
  - scsi: mpt3sas: Don't change DMA mask while reallocating pools
  - scsi: mpt3sas: re-do lost mpt3sas DMA mask fix
  - scsi: mpt3sas: Remove usage of dma_get_required_mask() API

That is a very intrusive backport (488 insertions, 284 deletions) with
significant conflict resolution due to the 3.10 kernel base.

This patch takes a simpler approach: remove the
_base_change_consistent_dma_mask() call that widens the coherent mask
after RDPQ allocation.  Since _base_config_dma_addressing() already
sets coherent to 32-bit on the first call, simply not widening it keeps
ALL coherent allocations below 4 GB for the entire driver lifetime.
Streaming DMA (scatter-gather for data I/O) remains 63/64-bit, so
there is no performance impact on the data path.

Pros:
  - Minimal, obviously correct change (19 lines removed, 0 added)
  - Eliminates the entire class of 4 GB boundary bugs at once
  - No risk of regression from a large, conflict-heavy backport
  - Streaming DMA stays wide - no data path performance impact

Cons:
  - Coherent allocations are restricted to the first 4 GB of DMA
    address space.  On systems with very heavy 32-bit DMA pressure
    from multiple devices this could theoretically make allocation
    harder.  In practice mpt3sas coherent pools total a few MB -
    negligible for any real server.

https://virtuozzo.atlassian.net/browse/PSBM-161670
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 054e3c22f1afa..7404d8ee61ba6 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2747,17 +2747,6 @@ _base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc, struct pci_dev *pdev)
 	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
@@ -5046,14 +5035,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;
 
-- 
2.24.3