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

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 4/9/26 15:56, Konstantin Khorenko wrote:
> 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>
> ---
>  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 054e3c22f1af..7404d8ee61ba 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;
>  

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.