> --------- Original Message ---------
> Sender : Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>
> Date : 2022-03-28 23:34 (GMT+9)
> Title : Re: [PATCH] PCI: dwc: Modify the check about MSI DMA mask 32-bit
>
> From: 이왕석 <wangseok.lee@xxxxxxxxxxx>
> 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.
>
Hello, Alexander Lobakin
Thanks for your review.
You are right.
My concern is that case of trying to use 33+bit dma mappings on
64bit system.
It is about the call sequence of the functions related to dma
setting, not the operation of the dma_set_mask() function.
If dma_set_mask(33+) is performed before dw_pcie_host_init()
for using 33+bit dma mapping, following error occurs
in dma_map_single_attrs()
ex) DMA addr 0x0000000106b052c8+2 overflow
(mask ffffffff, bus limit 27fffffff)
dma_set_mask(33+) -> dw_pcie_host_init(): dma_set_mask(32) ->
dma_map_single_attrs() ->
error return in dma_direct_map_page():
because dma addr is 33+ but masking value is 32
So if the user has already set dma_mask to 33+ in order to use 33+,
i suggested to modify dma_set_mask(32) not to be called.
Please let me know your opinion.
Thank you.
>>
>> 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@xxxxxxxxxxx>
>> ---
>> 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
