Date:   Wed, 1 Feb 2023 16:54:55 +0300
From:   Evgenii Shatokhin <e.shatokhin@...ro.com>
To:     Robin Murphy <robin.murphy@....com>
CC:     <kernel-team@...roid.com>, Vidya Sagar <vidyas@...dia.com>,
        Christoph Hellwig <hch@...radead.org>,
        <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Jingoo Han <jingoohan1@...il.com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, <linux@...ro.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Will McVicker <willmcvicker@...gle.com>,
        Serge Semin <Sergey.Semin@...kalelectronics.ru>
Subject: Re: [PATCH v6 0/2] PCI: dwc: Add support for 64-bit MSI target
 addresses

On 31.01.2023 15:42, Robin Murphy wrote:
> 
> On 2023-01-31 12:29, Evgenii Shatokhin wrote:
>> Hi,
>>
>> On 26.08.2022 02:54, Will McVicker wrote:
>>> Hi All,
>>>
>>> I've update patch 2/2 to address Robin's suggestions. This includes:
>>>
>>>   * Dropping the while-loop for retrying with a 64-bit mask in favor of
>>>     retrying within the error if-statement.
>>>   * Using an int for the DMA mask instead of a bool and ternary
>>> operation.
>>>
>>> Thanks again for the reviews and sorry for the extra revision today!
>>> Hopefully this is the last one :) If not, I'd be fine to submit patch 
>>> 1/2
>>> without 2/2 to avoid resending patch 1/2 for future revisions of patch
>>> 2/2
>>> (unless I don't need to do that anyway).
>>
>> The first patch of the series made it into the mainline kernel, but, it
>> seems, the second one ("PCI: dwc: Add support for 64-bit MSI target
>> address") did not. As of 6.2-rc6, it is still missing.
>>
>> Was it intentionally dropped because of some issues or, perhaps, just by
>> accident? If it was by accident, could you please queue it for inclusion
>> into mainline again?
> 
> Yes, it was dropped due to the PCI_MSI_FLAGS_64BIT usage apparently
> being incorrect, and some other open debate (which all happened on the
> v5 thread):
> 
> https://lore.kernel.org/linux-pci/YzVTmy9MWh+AjshC@lpieralisi/

I see. If I understand it correctly, the problem was that 
PCI_MSI_FLAGS_64BIT flag did not guarantee that 64-bit mask could be 
used for that particular allocation. Right?

> 
> The DMA mask issues have now been sorted out, 

I suppose, you mean 
https://lore.kernel.org/all/20230113171409.30470-26-Sergey.Semin@baikalelectronics.ru/?

It still breaks our particular case when the SoC has no 
32-bit-addressable RAM. We'd set DMA masks to DMA_BIT_MASK(36) in the 
platform-specific driver before calling dw_pcie_host_init(). However, 
dw_pcie_msi_host_init() resets it to 32-bit, tries dmam_alloc_coherent() 
and fails.

With 36-bit masks, the kernel seems to play well with the devices in our 
case.

I saw your comment in 
https://lore.kernel.org/linux-pci/4dc31a63-00b1-f379-c5ac-7dc9425937f4@arm.com/ 
that drivers should always explicitly set their masks.

Is it a really bad idea to check the current coherent mask's bits in 
dw_pcie_msi_host_init() and if it is more than 32 - just issue a warning 
rather than reset it to 32-bit unconditionally? That would help in our 
case. Or, perhaps, there is a better workaround.

Looking forward to your comments.


> so you, or Will, or anyone
> else interested should be free to rework this on top of linux-next
> (although at this point, more realistically on top of 6.3-rc1 in a few
> weeks).
> 
> Thanks,
> Robin.
> 
>> Support for 64-bit MSI target addresses is needed for some of our SoCs.
>> I ran into a situation when there was no available RAM in ZONE_DMA32
>> during initialization of PCIe host. Hence, dmam_alloc_coherent() failed
>> in dw_pcie_msi_host_init() and initialization failed with -ENOMEM:
>>
>> [    0.374834] dw-pcie 4000000.pcie0: host bridge /soc/pcie0@...0000
>> ranges:
>> [    0.375813] dw-pcie 4000000.pcie0:      MEM
>> 0x0041000000..0x004fffffff -> 0x0041000000
>> [    0.376171] dw-pcie 4000000.pcie0:   IB MEM
>> 0x0400000000..0x07ffffffff -> 0x0400000000
>> [    0.377914] dw-pcie 4000000.pcie0: Failed to alloc and map MSI data
>> [    0.378191] dw-pcie 4000000.pcie0: Failed to initialize host
>> [    0.378255] dw-pcie: probe of 4000000.pcie0 failed with error -12
>>
>> Mainline kernel 6.2-rc6 was used in that test.
>>
>> The hardware supports 64-bit target addresses, so the patch "PCI: dwc:
>> Add support for 64-bit MSI target address" should help with this
>> particular failure.
>>
>>
>>>
>>> Thanks,
>>> Will
>>>
>>> Will McVicker (2):
>>>    PCI: dwc: Drop dependency on ZONE_DMA32
>>>
>>> v6:
>>>   * Retrying DMA allocation with 64-bit mask within the error
>>> if-statement.
>>>   * Use an int for the DMA mask instead of a bool and ternary operation.
>>>
>>> v5:
>>>   * Updated patch 2/2 to first try with a 32-bit DMA mask. On failure,
>>>     retry with a 64-bit mask if supported.
>>>
>>> v4:
>>>   * Updated commit descriptions.
>>>   * Renamed msi_64b -> msi_64bit.
>>>   * Dropped msi_64bit ternary use.
>>>   * Dropped export of dw_pcie_msi_capabilities.
>>>
>>> v3:
>>>    * Switched to a managed DMA allocation.
>>>    * Simplified the DMA allocation cleanup.
>>>    * Dropped msi_page from struct dw_pcie_rp.
>>>    * Allocating a u64 instead of a full page.
>>>
>>> v2:
>>>    * Fixed build error caught by kernel test robot
>>>    * Fixed error handling reported by Isaac Manjarres
>>>   PCI: dwc: Add support for 64-bit MSI target address
>>>
>>>   .../pci/controller/dwc/pcie-designware-host.c | 43 +++++++++----------
>>>   drivers/pci/controller/dwc/pcie-designware.c  |  8 ++++
>>>   drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
>>>   3 files changed, 30 insertions(+), 23 deletions(-)
>>>
>>>
>>> base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
>>
>> Thank you in advance.
>>
>> Regards,
>> Evgenii
>>
>>
>>
> 

Powered by blists - more mailing lists