Message-ID: <46ba97c9-85ff-eb47-0d05-79dc3960d7b4@yadro.com>
Date:   Wed, 1 Feb 2023 16:54:55 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v6 0/2] PCI: dwc: Add support for 64-bit MSI target
 addresses
Content-Language: en-US
To:     Robin Murphy <robin.murphy@arm.com>
CC:     <kernel-team@android.com>, Vidya Sagar <vidyas@nvidia.com>,
        Christoph Hellwig <hch@infradead.org>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
        Jingoo Han <jingoohan1@gmail.com>,
        Rob Herring <robh@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>, <linux@yadro.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Will McVicker <willmcvicker@google.com>,
        Serge Semin <Sergey.Semin@baikalelectronics.ru>
References: <20220825235404.4132818-1-willmcvicker@google.com>
 <decae9e4-3446-2384-4fc5-4982b747ac03@yadro.com>
 <c014b074-6d7f-773b-533a-c0500e239ab8@arm.com>
From:   Evgenii Shatokhin <e.shatokhin@yadro.com>
In-Reply-To: <c014b074-6d7f-773b-533a-c0500e239ab8@arm.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@4000000
>> 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
>>
>>
>>
>