Re: [PATCH] PCI: dwc: Modify the check about MSI DMA mask 32-bit

[Alexander Lobakin <alexandr.lobakin@intel.com>]

From: 이왕석 <wangseok.lee@samsung.com>
Date: Mon, 28 Mar 2022 11:30:09 +0900

> If dma_mask is more than 32 bits this will trigger an error occurs when
> dma_map_single_attrs() is performed.
> 
> dma_map_single_attrs() -> dma_map_page_attrs()->
> error return in dma_direct_map_page().
> 
> On ARTPEC-8, this fails with:
> artpec8-pcie 17200000.pcie: DMA addr 0x0000000106b052c8+2 overflow
> (mask ffffffff, bus limit 27fffffff)

Isn't it a bug in the platform DMA code? dma_set_mask(32)
explicitly says that the system *must not* give DMA addresses wider
than 32 bits. If the system can't satisfy this requirement, then it
should return failure on dma_set_mask(32) -- this way you will only
get the corresponding warning, but there'll be no overflows (as the
mask will not be changed).
The idea of this call is to try to avoid getting 33+ bit mappings
so that PCI controllers which support only 32-bit masks could still
work correctly on the 64-bit systems. If the call fails, then this
message gets printed that you've been warned and it's your
responsibility to make sure that the controller won't get truncated
addresses. Having the call succeeded and then 33+ bit DMA addresses
is wrong.

Please correct me if I'm wrong.

> 
> There is no sequence that re-sets dev->dma_mask to more than 32 bits
> before call dma_map_single_attrs().
> The dev->dma_mask is first set just prior to the dw_pcie_host_init() call.
> Therefore, the check logic was modified to be performed only when
> the dev-dma_mask is not set larger than 32 bits.
> 
> Always setting dma_mask to 32 bits is not always correct,
> for example the ARTPEC-8 is an arm64 platform, and can access
> more than 32 bits
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 2fa86f3..7e25958 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -388,9 +388,11 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  							    dw_chained_msi_isr,
>  							    pp);
>  
> -			ret = dma_set_mask(pci->dev, DMA_BIT_MASK(32));
> -			if (ret)
> -				dev_warn(pci->dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
> +			if (!(*dev->dma_mask &  ~(GENMASK(31, 0)))) {
> +				ret = dma_set_mask(pci->dev, DMA_BIT_MASK(32));
> +					if (ret)
> +						dev_warn(pci->dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
> +			}
>  
>  			pp->msi_data = dma_map_single_attrs(pci->dev, &pp->msi_msg,
>  						      sizeof(pp->msi_msg),
> -- 
> 2.9.5

Thanks,
Al